[asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier

[Corey Farrell]

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


Could this be solved by: read(ast_consock, buf, sizeof(buf) - 1) ?

read() operates on buffers not strings, so it has no concept of null termination.  Also I think we need to either memset(buf, 0, sizeof(buf)) before the read, or (better) use the result of read to set the NULL terminator.  It looks like we're currently relying on buf to be zero filled before read() when it's actually uninitialized data.

I don't have a specific problem with increasing the buffer from 80 to 256, but I think we need to fix the parser so it doesn't crash if given 256 or more bytes.  I think this issue applies to 11+.

- Corey Farrell


On Nov. 13, 2014, 3:31 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4182/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2014, 3:31 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When connecting to the remote console, an identifier string is first provided that consists of hostname/pid/version.  This is parsed by the remote instance in a buffer allocated to only 80 bytes.  It is possible for a combination of very long hostname and very long asterisk version number to be greater than 80 characters, causing the parsing to fall off the end of the allocated memory buffer and potentially crash.
> 
> This change increases the buffer from 80 to 256 to significantly reduce that possibility.
> 
> 
> Diffs
> -----
> 
>   /branches/13/main/asterisk.c 427813 
> 
> Diff: https://reviewboard.asterisk.org/r/4182/diff/
> 
> 
> Testing
> -------
> 
> It stopped crashing on a repeated test I was running where the atoi of the version # happen to hit the end of the buffer.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141113/d59a55d8/attachment-0001.html>