[Home]

Summary:ASTERISK-14725: [patch] asterisk 1.6.2.0-beta4 crash when including nonexistent file from /etc/asterisk/manager.conf
Reporter:Alex Villacís Lasso (a_villacis)Labels:
Date Opened:2009-08-27 14:18:08Date Closed:2009-08-27 16:28:15
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Core/ManagerInterface
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-1.6.2.0-beta4-manager-fix-crash-on-include-nonexistent-file.patch
Description:If /etc/asterisk/manager.conf #includes a file that does not exist, asterisk will crash instead of continuing gracefully or logging the issue and exiting.

To reproduce:

Compile and install asterisk-1.6.2.0-beta4
Edit /etc/asterisk/manager.conf to #include any filename that does not exist.
(Re)start asterisk.

Expected result:
asterisk should either log the issue and continue, or exit gracefully with the logged issue.

Actual result:
SIGSEGV

When run from valgrind, the following is reported right before the crash:


==17755==
==17755== Invalid read of size 8
==17755==    at 0x456319: ast_variable_browse (config.c:400)
==17755==    by 0x4987EE: __init_manager (manager.c:4111)
==17755==    by 0x431AA9: main (asterisk.c:3600)
==17755==  Address 0x16 is not stack'd, malloc'd or (recently) free'd

When run under gdb, asterisk shows this:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000456319 in ast_variable_browse (config=0xfffffffffffffffe, category=0x517d06 "general") at config.c:400
400 if (category && config->last_browse && (config->last_browse->name == category)) {
(gdb) bt
#0  0x0000000000456319 in ast_variable_browse (config=0xfffffffffffffffe, category=0x517d06 "general") at config.c:400
#1  0x00000000004987ef in __init_manager (reload=<value optimized out>) at manager.c:4111
#2  0x0000000000431aaa in main (argc=<value optimized out>, argv=0x7fff11d93348) at asterisk.c:3600
(gdb) bt full
#0  0x0000000000456319 in ast_variable_browse (config=0xfffffffffffffffe, category=0x517d06 "general") at config.c:400
cat = <value optimized out>
#1  0x00000000004987ef in __init_manager (reload=<value optimized out>) at manager.c:4111
ucfg = <value optimized out>
cfg = (struct ast_config *) 0xfffffffffffffffe
val = <value optimized out>
cat = <value optimized out>
newhttptimeout = <value optimized out>
have_sslbindaddr = 0
hp = <value optimized out>
ahp = {hp = {h_name = 0x0, h_aliases = 0x0, h_addrtype = 0, h_length = 0, h_addr_list = 0x0},
 buf = '\0' <repeats 589 times>, "%\000\000\000\000*", '\0' <repeats 48 times>, "[\000]", '\0' <repeats 29 times>, "{|}", '\0' <repeats 130 times>, "\\\fE", '\0' <repeats 29 times>, "@\207v", '\0' <repeats 53 times>, "core show channeltype", '\0' <repeats 68 times>, "?\t\000\000?`M\000\000\000\000\000\000\000\000?\210v\000\000\000\000\000\002", '\0' <repeats 14 times>}
user = <value optimized out>
var = <value optimized out>
config_flags = <value optimized out>
__PRETTY_FUNCTION__ = "__init_manager"
#2  0x0000000000431aaa in main (argc=<value optimized out>, argv=0x7fff11d93348) at asterisk.c:3600
c = <value optimized out>
filename = "/root/.asterisk_history", '\0' <repeats 56 times>
hostname = "rpmbuild64-2.elastix.palosanto.com", '\0' <repeats 29 times>
tmp = "\001\000\000\000\000\000\000\000?\031?U:\000\000\000?1?\021?\177\000\000@5?\001\000\000\000\000??\001\000\000\000\000\000)\000\000\000\000\000\000\000?\031?U:\000\000\000?<?U:\000\000\000\2201?\021?\177\000\000\2211?\021?\177\000"
xarg = 0x0
x = <value optimized out>
f = <value optimized out>
sigs = {__val = {134238211, 0 <repeats 15 times>}}
num = 58
buf = <value optimized out>
runuser = 0x0
rungroup = 0x0
remotesock = 0x0
__PRETTY_FUNCTION__ = "main"
__func__ = "main"
(gdb)



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

CentOS 5.3/Elastix on VirtualBox 64 bits

Linux rpmbuild64-2.elastix.palosanto.com 2.6.18-128.4.1.el5 #1 SMP Tue Aug 4 20:19:25 EDT 2009 x86_64 x86_64 x86_64 GNU/Linux

Comments:By: Alex Villacís Lasso (a_villacis) 2009-08-27 14:20:18

FreePBX creates a manager.conf file that #includes a manager_additional.conf file that does not exist. Therefore, using FreePBX with this version of asterisk is likely to crash asterisk as shown above.
By: Alex Villacís Lasso (a_villacis) 2009-08-27 14:26:15

From the sources, I see that the program crashes on access to cfg which is set to an invalid pointer value of 0xfffffffffffffffe. This happens to be CONFIG_STATUS_FILEINVALID as defined by (void *)-2 .

The function ast_config_load2 which is called by main/manager.c in line 4088 returns CONFIG_STATUS_FILEINVALID. However the return value is not checked for this, just for NULL and CONFIG_STATUS_FILEUNCHANGED. Then the pointer is passed around as if it were valid, with the resulting crash.
By: Alex Villacís Lasso (a_villacis) 2009-08-27 15:56:48

I have submitted a patch to check for CONFIG_STATUS_FILEINVALID. With this patch, asterisk no longer crashes, but instead continues with AMI disabled.
By: Digium Subversion (svnbot) 2009-08-27 16:27:16

Repository: asterisk
Revision: 214514

U   trunk/main/manager.c

------------------------------------------------------------------------
r214514 | tilghman | 2009-08-27 16:27:16 -0500 (Thu, 27 Aug 2009) | 7 lines

Ensure that we check for the special value CONFIG_STATUS_FILEINVALID.
(closes issue ASTERISK-14725)
Reported by: a_villacis
Patches:
      asterisk-1.6.2.0-beta4-manager-fix-crash-on-include-nonexistent-file.patch uploaded by a villacis (license 660)
      (Plus a few of my own, to catch the remaining places within manager.c where it could have been a problem)

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

http://svn.digium.com/view/asterisk?view=rev&revision=214514
By: Digium Subversion (svnbot) 2009-08-27 16:28:14

Repository: asterisk
Revision: 214515

_U  branches/1.6.2/
U   branches/1.6.2/main/manager.c

------------------------------------------------------------------------
r214515 | tilghman | 2009-08-27 16:28:14 -0500 (Thu, 27 Aug 2009) | 14 lines

Merged revisions 214514 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
 r214514 | tilghman | 2009-08-27 16:26:37 -0500 (Thu, 27 Aug 2009) | 7 lines
 
 Ensure that we check for the special value CONFIG_STATUS_FILEINVALID.
 (closes issue ASTERISK-14725)
  Reported by: a_villacis
  Patches:
        asterisk-1.6.2.0-beta4-manager-fix-crash-on-include-nonexistent-file.patch uploaded by a villacis (license 660)
        (Plus a few of my own, to catch the remaining places within manager.c where it could have been a problem)
........

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

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