[asterisk-dev] [Code Review] Segmentation fault in strlen () from /lib64/libc.so.6

[wdoekes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1225/#review3596
-----------------------------------------------------------


I could be wrong, but the way I read parse_uri (in reqresp_parser), the only time that domain comes out NULL, is when ast_strlen_zero(uri).

So .. I'd move the return -1 to the if-check that already exists above.

- wdoekes


On 2011-05-23 09:01:59, jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1225/
> -----------------------------------------------------------
> 
> (Updated 2011-05-23 09:01:59)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and David Vossel.
> 
> 
> Summary
> -------
> 
> According to the backtrace, __set_address_from_contact is invoking ast_sockaddr_resolve (via a couple of wrappers) in a way that is causing a segfault when it tries to perform ast_strdup on the str value this function receives.  At the same time, it is known that the SIP URI is failing to parse properly.  It appears that the improper URI is parsed, lacks the appropriate data, and fails to fill in an expected field (domain) sent in from __set_address_from_contact (chan_sip.c).  Then this domain is used as the str value for ast_sockaddr_resolve, which ultimately causes the segfault.
> 
> The submitted patch's approach was to simply return a failure when this str value was null from ast_sockaddr_resolve, but I feel that probably obfuscates the problem.  For now, I've opted to deal with it at the the level of the invoking function.  ast_sockaddr_resolve should never be invoked with a null value for str.
> 
> 
> I noticed some peculiar comments around where I ended up putting the patch in though, so a little caution is probably a good thing.  I don't see anyway for this to mess things up, but I could be wrong.
> 
> 
> This addresses bug 18857.
>     https://issues.asterisk.org/view.php?id=18857
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 319995 
> 
> Diff: https://reviewboard.asterisk.org/r/1225/diff
> 
> 
> Testing
> -------
> 
> Unfortunately, I don't know a proper test setup for this situation.  I was thinking sipp with a deliberately botched message, but I don't know what sort of dialog invokes __set_address_from_contact.
> 
> I'm going to ask the reporter to test the patch though and see if he ever receives the warning message I've added.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110523/d2b2bab5/attachment.htm>