[Kamailio-Devel] [SR-Dev] content length

[Daniel-Constantin Mierla]

On 03/30/2009 10:21 PM, Andrei Pelinescu-Onciul wrote:
> On Mar 30, 2009 at 21:48, Daniel-Constantin Mierla <miconda at gmail.com> wrote:
>   
>> On 03/30/2009 06:27 PM, Juha Heinanen wrote:
>>     
>>> Andrei Pelinescu-Onciul writes:
>>>
>>>       
>>>> It's not a bug in the UA, it's a bug in the proxy code that uses a
>>>> Content-Length received from the network without checking if it's
>>>> valid.
>>>>         
>>> if that is the case, then i agree with you.  proxy code should not do
>>> such thing and if it does t is clearly a bug in the proxy code.  i
>>> wonder in how many places k currently trusts content-length.
>>>  
>>>       
>> This trust of content-length needs be fixed I agree. However it looks to 
>> me too radical to call abort() on purpose. A developer can fix that 
>> quickly, but users having deployed the sip router cannot coper properly 
>> with. Like in buffer overflow cases, the code detects the case and 
>> returns error, does not call abort(). I see this being similar. I would 
>> avoid abort() on purpose anywhere at runtime, but write error messages, 
>> avoid crash and keep running.
>>     
>
> If the abort() wouldn't have been there, you wouldn't have discovered
> this bug.
>   
well, this is questionable, lot of bugs are reported by error messages 
in syslog.
> In general abort() is used only for important bugs and one shouldn't
> expect the proxy to survive using the api in the wrong way.
> We could try to workaround SIGSEGV too, but it's much better to let it
> coredump.
>
> What can we do is to use some define, e.g.:
> #ifndef RELEASE
> abort()
> #endif
>   

maybe this is better, a sr_abort(code) marcro that does either "abort()" 
or "return code" depending of compile mode (release or not).


> but this still would have delayed finding this bug a lot.
>   
The bug was identified manly by the syslog message printed before the 
abort(), not by the crash ...

Cheers,
Daniel

-- 
Daniel-Constantin Mierla