[SR-Dev] content length

[Andrei Pelinescu-Onciul]

On Mar 30, 2009 at 17:46, Daniel-Constantin Mierla <miconda at gmail.com> wrote:
> Hello,
> 
> On 03/30/2009 05:24 PM, Andrei Pelinescu-Onciul wrote:
> >On Mar 30, 2009 at 16:06, Daniel-Constantin Mierla <miconda at gmail.com> 
> >wrote:
> >  
> >>Hello,
> >>
> >>the data lumps system is critically affected if content-length is wrong 
> >>for UDP. The anchor_lump() calls abort() in the case content-length is 
> >>higher than actually body length.
> >>    
> >
> >It's true that anchor_lump() calls abort if the offset passed to it is
> >outside the message, but I don't see where anchor_lump() is called with
> >a value depending on Content-Length (at least in sip-router and ser).
> >  
> I haven't checked the modules in ser, just the data_lump.c file for 
> anchor_lump() in sip-router core. In k most of the modules takes the 
> length from header. Probably a wrapper that corrects it would be good.
> 
> The issue remains, is this a case of runtime abort()?

It's an abort() to quickly catch bugs (the content length value should
always be checked and _never_ trusted) and to force people to fix them.

We could eliminate the abort() but then the incentive for fixing the
real bug will be reduced :-)
> 
> >  
> >>This can be prevented by called sanity 
> >>module to check the content length, however, I consider being too 
> >>drastic to have abort in such case, it would be better to return an 
> >>error and let the sip router process other messages. Opinions?
> >>
> >>Checking sip-router sources, it faces same issue.
> >>
> >>Another option would be to correct the C-L value locally, but the right 
> >>one is that phone vendor fixes its side.
> >>    
> >
> >It's fixed automatically in sip-router, if the destination protocol is
> >tcp or tls.
> >  
> Others reaction was to 400-reply the request, being a basic bug that 
> should be fixed in uac side...

It costs us very little to fix it and so I think it's better to be nice,
even with broken UACs.
If one wants to drop them, it can always use the sanity module.

Andrei