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

[Mark Murawski]

I have an issue posted and also have a fix to this problem in a 
different way.  A sanity check in netsock2 as well as making sure all 
the parameters to be parsed into are initalized.

https://issues.asterisk.org/view.php?id=19346


On 05/23/11 11:05, wdoekes wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1225/
>
>
> 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 May 23rd, 2011, 9:01 a.m., jrose wrote:
>
> Review request for Asterisk Developers, Russell Bryant and David Vossel.
> By jrose.
>
> /Updated 2011-05-23 09:01:59/
>
>
>   Description
>
> 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.
>
>
>   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.
>
> *Bugs: * 18857 <https://issues.asterisk.org/view.php?id=18857>
>
>
>   Diffs
>
>     * /branches/1.8/channels/chan_sip.c (319995)
>
> View Diff <https://reviewboard.asterisk.org/r/1225/diff/>
>
>
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>     http://lists.digium.com/mailman/listinfo/asterisk-dev