[asterisk-dev] [Code Review]: Fix r374905 misuse of strdupa

[wdoekes]

> On Oct. 12, 2012, 4:25 p.m., rmudgett wrote:
> >

Thanks for the quick review.


> On Oct. 12, 2012, 4:25 p.m., rmudgett wrote:
> > /branches/1.8/channels/chan_sip.c, lines 2678-2679
> > <https://reviewboard.asterisk.org/r/2162/diff/2/?file=31859#file31859line2678>
> >
> >     I think strlen + 1 for terminator is needed.

You're absolutely right.


> On Oct. 12, 2012, 4:25 p.m., rmudgett wrote:
> > /branches/1.8/channels/chan_sip.c, lines 2696-2697
> > <https://reviewboard.asterisk.org/r/2162/diff/2/?file=31859#file31859line2696>
> >
> >     Duplicate strcasestr here.

Fixed.


> On Oct. 12, 2012, 4:25 p.m., rmudgett wrote:
> > /branches/1.8/channels/chan_sip.c, lines 2698-2700
> > <https://reviewboard.asterisk.org/r/2162/diff/2/?file=31859#file31859line2698>
> >
> >     The sizeof("string") count includes the null terminator so I think you are advancing too far.

This is where I whisper something about not copying my tests verbatim ;)


- wdoekes


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


On Oct. 12, 2012, 3:40 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2162/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2012, 3:40 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> I was a bit quick on the ship-it there ;)
> Those strdupa's are far from safe from direct user input.
> 
> Here's a fix, which happens to slightly faster too.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 374930 
> 
> Diff: https://reviewboard.asterisk.org/r/2162/diff
> 
> 
> Testing
> -------
> 
> Got asterisk to crash easily before patch. No crash after. Mark's test from r2123 still completes with 0 errors.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121013/424ca1dd/attachment-0001.htm>