[Home]

Summary:ASTERISK-17630: [patch] Concatenates uninitialized buffer causes garbage data prior result also may cause crash
Reporter:John Zhong (johnz)Labels:
Date Opened:2011-03-31 18:37:47Date Closed:2011-04-22 09:35:18
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Functions/func_shell
Versions:1.8.3 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 0019050.diff
Description:Noticed SHELL function returns garbage data ahead expected result.

To Reproduce:
1) Run a TCP dump tool on either Asterisk server side or FastAGI client side
( Most TCP dump tool does not work well when two peers are on same machine )

2) Write a simple FastAGI program, it only issues:

GET VARIABLE SHELL("echo -n hello")

3) Modify the extensions.conf by adding an extension to point to the FastAGI client

4) Make a call to that extension, check the TCP dump result.

I noticed:

32 30 30 20 72 65 73 75 6C 74 3D 31 20 28 98 ED AD B6 B8 C8 19 68 65 6C 6C 6F 29 0A
200 result=1 (?í­¶¸È.hello).


****** ADDITIONAL INFORMATION ******

After checkout the source code, noticed the bug, there is the patch.
I tested the fix, it works great.

Changelog:
- Fixs garbage data being concatenated into result, accessing
  uninitialized string may cause crash issue
- Fixs multiple line result of SHELL breaks AGI parse issue


<inline patch removed by lmadsen/>
Comments:By: Digium Subversion (svnbot) 2011-04-22 08:58:05

Repository: asterisk
Revision: 314778

U   branches/1.6.2/res/res_agi.c

------------------------------------------------------------------------
r314778 | russell | 2011-04-22 08:58:04 -0500 (Fri, 22 Apr 2011) | 11 lines

Initialize buffers in getvar and getvarfull.

Initialize the buffers used to hold the result from GET VARIABLE or
GET VARIABLE FULL.  The bug report shows func_read returning garbage in
the result.  It assumed that the buffer passed in was initialized, like many
other functions do.  In the more common code path (through the dialplan), it
is initialized, so just initialize it here too.

(closes issue ASTERISK-17630)
Reported by: johnz

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=314778
By: Digium Subversion (svnbot) 2011-04-22 09:02:24

Repository: asterisk
Revision: 314780

_U  branches/1.8/
U   branches/1.8/res/res_agi.c

------------------------------------------------------------------------
r314780 | russell | 2011-04-22 09:02:23 -0500 (Fri, 22 Apr 2011) | 18 lines

Merged revisions 314778 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

........
 r314778 | russell | 2011-04-22 08:58:03 -0500 (Fri, 22 Apr 2011) | 11 lines
 
 Initialize buffers in getvar and getvarfull.
 
 Initialize the buffers used to hold the result from GET VARIABLE or
 GET VARIABLE FULL.  The bug report shows func_read returning garbage in
 the result.  It assumed that the buffer passed in was initialized, like many
 other functions do.  In the more common code path (through the dialplan), it
 is initialized, so just initialize it here too.
 
 (closes issue ASTERISK-17630)
 Reported by: johnz
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=314780
By: Digium Subversion (svnbot) 2011-04-22 09:08:03

Repository: asterisk
Revision: 314781

_U  trunk/
U   trunk/res/res_agi.c

------------------------------------------------------------------------
r314781 | russell | 2011-04-22 09:08:03 -0500 (Fri, 22 Apr 2011) | 25 lines

Merged revisions 314780 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r314780 | russell | 2011-04-22 09:02:23 -0500 (Fri, 22 Apr 2011) | 18 lines
 
 Merged revisions 314778 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ........
   r314778 | russell | 2011-04-22 08:58:03 -0500 (Fri, 22 Apr 2011) | 11 lines
   
   Initialize buffers in getvar and getvarfull.
   
   Initialize the buffers used to hold the result from GET VARIABLE or
   GET VARIABLE FULL.  The bug report shows func_read returning garbage in
   the result.  It assumed that the buffer passed in was initialized, like many
   other functions do.  In the more common code path (through the dialplan), it
   is initialized, so just initialize it here too.
   
   (closes issue ASTERISK-17630)
   Reported by: johnz
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=314781
By: Digium Subversion (svnbot) 2011-04-22 09:34:25

Repository: asterisk
Revision: 314822

U   branches/1.4/res/res_agi.c

------------------------------------------------------------------------
r314822 | russell | 2011-04-22 09:34:25 -0500 (Fri, 22 Apr 2011) | 11 lines

Initialize buffers in getvar and getvarfull.

Initialize the buffers used to hold the result from GET VARIABLE or
GET VARIABLE FULL.  The bug report shows func_read returning garbage in
the result.  It assumed that the buffer passed in was initialized, like many
other functions do.  In the more common code path (through the dialplan), it
is initialized, so just initialize it here too.

(closes issue ASTERISK-17630)
Reported by: johnz

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=314822
By: Digium Subversion (svnbot) 2011-04-22 09:35:18

Repository: asterisk
Revision: 314823

_U  branches/1.6.2/

------------------------------------------------------------------------
r314823 | russell | 2011-04-22 09:35:17 -0500 (Fri, 22 Apr 2011) | 18 lines

Recorded merge of revisions 314822 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r314822 | russell | 2011-04-22 09:34:23 -0500 (Fri, 22 Apr 2011) | 11 lines
 
 Initialize buffers in getvar and getvarfull.
 
 Initialize the buffers used to hold the result from GET VARIABLE or
 GET VARIABLE FULL.  The bug report shows func_read returning garbage in
 the result.  It assumed that the buffer passed in was initialized, like many
 other functions do.  In the more common code path (through the dialplan), it
 is initialized, so just initialize it here too.
 
 (closes issue ASTERISK-17630)
 Reported by: johnz
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=314823