[asterisk-dev] [Code Review] Fix misuses of ast_str

[rmudgett]

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

Ship it!


Compilers don't lie.

- rmudgett


On Oct. 12, 2012, 1:41 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2161/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2012, 1:41 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> While writing the TCP tests for https://reviewboard.asterisk.org/r/2123 I accidentally misused ast_str when writing the test. I corrected my error and then realized it may be being misused other places in the code. Here's an example of the misuse I'm referring to:
> 
> int foo(struct ast_str *str)
> {
>     ast_str_set(&str, 0, "%s", some_string);
> }
> 
> int bar(void)
> {
>     struct ast_str *str = ast_str_create(32);
> 
>     foo(str);
> 
>     do_something(str); /* DANGEROUS */
> }
> 
> So let's go through this.
> 1) bar() creates an ast_str and calls foo(). 
> 2) foo() sets the ast_str to some value using ast_str_set(). 
> 3) ast_str_set() takes a pointer to the ast_str pointer because the ast_str can be reallocated. If this reallocation occurs, then when control returns to foo(), its version of str is pointing to invalid data.
> 
> The fix is to either 
> 
> 1) Pass a struct (or other container) with the ast_str pointer to foo()
> 2) Pass a pointer to the ast_str pointer to foo()
> 
> In the cases changed in this diff, the second approach was taken since the ast_str pointers did not already belong to some struct.
> 
> While there are changes in app_dial.c, chan_iax2.c, and ccss.c, the only one that can be problematic in its current state is ccss.c. This is because in app_dial and chan_iax2, the ast_strs are created using ast_str_alloca() instead of ast_str_create(). This means they cannot be reallocated by a call to ast_str_set(). I elected to make the changes in app_dial and chan_iax2 because the functions as they exist have the potential for danger and should be made safer.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/apps/app_dial.c 374904 
>   /branches/1.8/channels/chan_iax2.c 374904 
>   /branches/1.8/main/ccss.c 374904 
> 
> Diff: https://reviewboard.asterisk.org/r/2161/diff
> 
> 
> Testing
> -------
> 
> Compiles, brah!
> 
> 
> Thanks,
> 
> Mark
> 
>

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