[Home]

Summary:ASTERISK-17360: [patch] Null values in format strings lead to segfault
Reporter:Ben Klang (bklang)Labels:
Date Opened:2011-02-07 07:32:28.000-0600Date Closed:2011-06-07 14:04:59
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/Portability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) null-strings.patch
Description:This is an issue I have reported in several other places and which have been fixed.  Instead of opening a single ticket for these three I am putting them into a single ticket.

res/res_jabber.c:2493 found->description
channels/chan_sip.c:23429 authpeer->name
res/res_fax.c:1770

The first two I have patched trivially and attached it here.  For res_fax.c I'm not sure which variable was null and whether protection should be applied to multiple variables so I do not have a patch for this.

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

res_fax.c segfault backtrace:

#0  0xfeca47a0 in countbytes () from /usr/lib/libc.so.1
#1  0xfecf0793 in _ndoprnt () from /usr/lib/libc.so.1
#2  0xfecf31bd in vsnprintf () from /usr/lib/libc.so.1
#3  0x08165079 in __ast_str_helper (buf=0xfa1ea64c, max_len=0, append=1,
   fmt=0xfde517e8 "Channel: %s\r\nContext: %s\r\nExten: %s\r\nCallerID: %s\r\nRemoteStationID: %s\r\nLocalStationID: %s\r\nPagesTransferred: %s\r\nResolution: %s\r\nTransferRate: %s\r\nFileName: %s\r\n",
   ap=0xfa1ea6b4 "jx?\b?\036?\024?\036?d?\036?\230\021?\b") at strings.c:72
#4  0x0811e24f in __ast_manager_event_multichan (category=2,
   event=0xfde5075c "ReceiveFAX", chancount=0, chans=0x0,
   file=0xfde5037e "res_fax.c", line=1770, func=0xfde50195 "receivefax_exec",
   fmt=0xfde517e8 "Channel: %s\r\nContext: %s\r\nExten: %s\r\nCallerID: %s\r\nRemoteStationID: %s\r\nLocalStationID: %s\r\nPagesTransferred: %s\r\nResolution: %s\r\nTransferRate: %s\r\nFileName: %s\r\n") at strings.h:787
ASTERISK-1  0xfde48e39 in receivefax_exec (chan=0x8abb808,
   data=0xfa1ecba4 "/var/spool/fax/aspenfunding-1297083624.16.fax")
   at res_fax.c:1749
ASTERISK-2  0x0812820d in pbx_exec (c=0x8abb808, app=0x857e5e8,
   data=0xfa1ecba4 "/var/spool/fax/aspenfunding-1297083624.16.fax")
   at strings.h:64
ASTERISK-3  0x08136287 in pbx_extension_helper (c=0x8abb808, con=0x0,
   context=0x8abbb74 "standard-fax", exten=0x8abbbc4 "fax", priority=2,
   label=0x0, callerid=0x86dc5b0 "7703222389", action=139978216,
   found=0xfa1ecba4, combined_find_spawn=1) at pbx.c:4085
ASTERISK-4  0x0813ca40 in __ast_pbx_run (c=0x8abb808, args=0x0) at pbx.c:4608
ASTERISK-5  0x0813f3c0 in pbx_thread (data=0x0) at pbx.c:5017
ASTERISK-6 0x08172af9 in dummy_start (data=0x0) at utils.c:973
ASTERISK-7 0xfed2cd66 in _thrp_setup () from /usr/lib/libc.so.1
ASTERISK-8 0xfed2cff0 in __csigsetjmp () from /usr/lib/libc.so.1
ASTERISK-9 0x00000000 in ?? ()
Comments:By: Leif Madsen (lmadsen) 2011-02-07 11:04:04.000-0600

Thanks for the patch! I am marking this as confirmed since some additional work appears to be required for res_fax but there is a patch here.

By: Andrew Latham (lathama) 2011-02-07 12:41:11.000-0600

I think I am hitting something close to this...  I will test the patch tonight...


Core was generated by `/usr/sbin/asterisk -g'.
Program terminated with signal 11, Segmentation fault.
#0  0x0000000000489585 in ast_strlen_zero (cmd=0x97642f0 "sip", cli_word=0x1e <Address 0x1e out of bounds>) at /usr/src/1.8.3/include/asterisk/strings.h:65
65              return (!s || (*s == '\0'));
#0  0x0000000000489585 in ast_strlen_zero (cmd=0x97642f0 "sip", cli_word=0x1e <Address 0x1e out of bounds>) at /usr/src/1.8.3/include/asterisk/strings.h:65
No locals.
#1  word_match (cmd=0x97642f0 "sip", cli_word=0x1e <Address 0x1e out of bounds>) at cli.c:1874

By: Jonathan Rose (jrose) 2011-03-18 09:43:04

@lathama:  I don't really believe that has anything to do with this particular bug.  ast_strlen_zero shouldn't have any problems with null pointers.



@bklang:  I've made a patch to perform your changes as well as protect all of the variables within messages printed in that function of res_fax.c  I'd like you to test it if you would, as I can't pinpoint any additional specific changes that should be needed in that particular section of code but at the same time I have no means of testing this module in Solaris.  According to my sources, these bugs are Solaris-specific.  All this protects is the chan->name across the various log statements in there (which is the only value involved int he log) and I find it weird in the first place that you'd be able to reach that function without a channel that has a name in it.  If the channel itself is what's null there, we might need to probe this issue a little more thoroughly.

Also, I'm going to remark that this fix seems to be something that just keeps it from crashing.  This situation which is causing these values to be null in the first place actually shouldn't be able to occur, and that's what really needs to be addressed probably.



By: Andrew Latham (lathama) 2011-03-18 09:46:20

jrose: Yes the issue turned out to be a config issue.  Too many hands in the cookie jar.



EDIT:  I got together with another dev and we decided the problem was actually quite a bit different... so I'm taking that patch back and we did something a bit different.



By: Digium Subversion (svnbot) 2011-03-18 11:19:09

Repository: asterisk
Revision: 311352

U   branches/1.8/channels/chan_sip.c
U   branches/1.8/res/res_fax.c
U   branches/1.8/res/res_jabber.c

------------------------------------------------------------------------
r311352 | jrose | 2011-03-18 11:19:08 -0500 (Fri, 18 Mar 2011) | 10 lines

Changes some print statements/events to use a blank string in place of NULL if the string in question is NULL.

This is supposed to improve Solaris compatibility since Solaris goes berserk when trying to output NULL strings.

(closes issue ASTERISK-17360)
Reported by: bklang
Patches:
     null-strings.patch uploaded by bklang (license 919)


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

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

By: Digium Subversion (svnbot) 2011-03-18 11:24:21

Repository: asterisk
Revision: 311373

_U  trunk/
U   trunk/channels/chan_sip.c
U   trunk/res/res_fax.c
U   trunk/res/res_jabber.c

------------------------------------------------------------------------
r311373 | jrose | 2011-03-18 11:24:20 -0500 (Fri, 18 Mar 2011) | 16 lines

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

........
 r311352 | jrose | 2011-03-18 11:19:05 -0500 (Fri, 18 Mar 2011) | 10 lines
 
 Changes some print statements/events to use a blank string in place of NULL if the string in question is NULL.
 
 This is supposed to improve Solaris compatibility since Solaris goes berserk when trying to output NULL strings.
 
 (closes issue ASTERISK-17360)
 Reported by: bklang
 Patches:
       null-strings.patch uploaded by bklang (license 919)
........

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

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

By: Jonathan Rose (jrose) 2011-03-18 11:26:49

@bklang:  Could I ask you to try out the latest version of trunk or 1.8 from SVN later on to see if this fixes your fax issues?  Also, I applied the rest of your patches with some very minor changes.  Thanks for your help.

Also, if it is still an issue, try manually setting the channel variable "LOCALSTATIONID" in the dialplan right before you start the fax request.  Matt Nicholson is currently theorizing that this is what was causing the problem.  Of course, any output and dialplan information would be appreciated in dealing with this bug.