[asterisk-dev] apps/app_page.c: fix buffer overflow and invalid memory access

[Alex Villací­s Lasso]

We at Palosanto Solutions have been shipping our asterisk build with 
this patch applied, but I consider this patch should be reviewed and 
possibly merged upstream, for both 1.4.x and 1.6.x series of Asterisk.

Problem description:

When defining a paging group, a list of extensions must be defined, 
separated with ampersands. Within the code, this list is tokenized and 
used to populate dialing contexts in a static array declared as a local 
variable of a function. This static array has a capacity of MAX_DIALS = 
128, but the code apparently has no verifications to ensure that the 
buffer is not being filled beyond its declared capacity. A client of 
ours declared a paging group with 130 extensions, and we saw the buffer 
overflowing and scribbling over the next few variables, including the 
variable that keeps track of the number of actual extensions. On context 
cleanup, the number of extensions is now invalid and leads to undefined 
behavior. In this particular client case, he experienced hangup of all 
calls, including calls on extensions not included in the page group.

While fixing this problem, we found a potential second problem: in the 
scenario in which the n-th dialing context cannot be created (for 
whatever reason), or if the n-th extension happens to be the very 
extension that is issuing the paging, the array position reserved for 
the n-th dialing context created remains uninitialized. In the cleanup 
loop, the code incorrectly assumes that the n-th position is always 
valid and points to a valid context. On the previously described 
scenario, this leads to invalid memory accesses, which may lead to 
Asterisk crashing.

The attached patch fixes both identified issues. This patch counts the 
number of extensions in the list and dynamically allocates enough memory 
for the actual number of extensions in the page group. As a side effect, 
the MAX_PAGE limit is now removed and the extension list can be 
arbitrarily long, memory permitting. Also the array positions are 
initialized to NULL before context allocation and checked for NULL on 
cleanup.


* apps/app_page.c
- Fix buffer overflow caused by attempts to define a page group with 
more than 128 extensions.
- Fix potential invalid memory access on cleanup of a dialing structure 
through an uninitialized pointer after failure to create at least one 
dialing structure.

-- 
perl -e '$x=2.4;print sprintf("%.0f + %.0f = %.0f\n",$x,$x,$x+$x);'

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080912-asterisk-app_page-fix-buffer-overflow.patch
Type: text/x-patch
Size: 2316 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20090110/2556eeda/attachment-0001.bin