Re: [reSIProcate] Parser validation issues
Contact "*" is a hack, but I grant your point. Not sure I agree that a
distinct type is warranted, but that is a reasonable solution.
As for the integer parsing, I agree it could/should be tightened. But
not sure the attack you propose is particularly scary; there is no
recursion.
thanks,
david
On 5/8/06, David Schwartz <David.Schwartz@xxxxxxxxxx> wrote:
Hi David.
The issue here IMHO is not one of conformity; rather it is about basic parser
behavior. A parser that does not conform to the ABNF is not being true to the
spec. A parser that allows a * in the From field for example merely because the
underlying implementation uses a NameAddr for both From and Contact is
problematic to say the least. The fact that at some later point an exception
may be thrown (in this case the validity check of To, From, etc. will catch
this error) is a hack. This should never pass the parser test.
I have raised this issue at the last repro coding party. We are running ahead
trying to implement all the latest and greatest features while neglecting the
grunt work of stabilizing and cleaning the code.
The * is but one example. If you look at the "parse" code of the NameAddr you will see that while
it handles the three scenarios ("display name" <, display name <, and <) it only throws
an exception for one of the three while the malformed code case for which the exception is thrown applies to
all three scenarios.
The issue with the Expires field is also much more severe than you think. The current
code recreates 'atoi' functionality while checking 'isDigit' recursively and multiplying
and adding by 10 each time until a non digit is reached. Malicious code in the form of
"Expires:
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001\r\n"
will not crash or throw an exception yet waste valuable CPU processing power and time
(TCP will not have MTU issue and UDP can be made just small enough to pass).
Until now I have spent very little time looking at the actual parser code and I
would like to believe that the issues that I did see and mention in this
posting are the only problematic ones.
I do think, however, that it is high time we spent a little more time hardening
the code before proceeding to add the next gen features.
Cheers,
David.
-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of david
Butcher
Sent: ב 08 מאי 2006 20:45
To: Ofir Roval
Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Parser validation issues
In general, the parser is not intended to enforce conformity. It is
required to parse all correct inputs, but is not required to reject
all incorrect inputs.
However, the integer overflow seems like it should be fixed.
david
On 5/8/06, Ofir Roval <Ofir.Roval@xxxxxxxxxx> wrote:
>
> Hi all,
>
> I came across 2 parsing issues that I would like to share:
>
> 1. When parsing NameAddr fields, the parser will always accept a header
> containing only a STAR sign (*) as valid value. This is of course the
> required behavior for Contact header but for headers like >From and To I
> believe this violates the RFC syntax. Note: other syntax violations will
> usually result with an exception.
>
> 2. When parsing an Interger field like Expires, an integer value greater
> that MAX_INT will result in integer-overflow and the actual parsed int may
> become a negative number.
>
> These issues led me to raise the following question: to what extent is the
> parser responsible for ensuring legal syntax ? what do you think ?
>
>
> Ofir Roval - Kayote Networks
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>
>
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel