[reSIProcate] Parser validation issues

[david Butcher]

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 at kayote.com> 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 at list.sipfoundry.org [mailto:resiprocate-devel-bounces at list.sipfoundry.org] On Behalf Of david Butcher
> Sent: ב 08 מאי 2006 20:45
> To: Ofir Roval
> Cc: resiprocate-devel at list.sipfoundry.org.
> 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 at kayote.com> 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 at list.sipfoundry.org
> > https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
> >
> >
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at list.sipfoundry.org
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>