[Home]

Summary:ASTERISK-12912: [patch] Using SIP_HEADER in AMI with NULL channel causes crash
Reporter:Makoto Dei (makoto)Labels:
Date Opened:2008-10-16 03:58:35Date Closed:2008-10-17 21:08:04
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) SIP_HEADER-NULL-channel.patch
Description:Connect to AMI, then send the following lines.
So asterisk will crash.

Action: GetVar
Channel:
Variable: SIP_HEADER(P-Called-Party-ID)

I have tested only on 1.2, but I believe that it happens on 1.4 or later.

Attached patch will fix the problem.
Comments:By: Leif Madsen (lmadsen) 2008-10-16 13:39:59

Can you please try and reproduce on 1.4 or later to verify if this is still a problem? I will try to reproduce here, but not sure if I will find the time in the next little bit.

Thanks!
By: Mark Michelson (mmichelson) 2008-10-16 17:44:23

Yes, I echo blitzrage in saying to please try this with 1.4. It appears that there are defenses in place which would prevent this crash from occurring. However, hard evidence is better than just saying that something "should" work, so if you can make this happen with 1.4, then we'll get the fix in.
By: Mark Michelson (mmichelson) 2008-10-16 17:56:46

Actually, seeing this in further detail, I realized that this is actually an easy thing to set up myself. I gave the test a shot, and sure enough, it crashed in 1.4, too!

So, on to your patch. It appears that this crash will happen with any dialplan function which does not check for a channel to be non-NULL. Instead of modifying chan_sip to not crash in func_header_read, it would be a much better idea to nip the problem in the bud, so to speak, by making sure not to call any dialplan functions at all if the channel is NULL.
By: Mark Michelson (mmichelson) 2008-10-16 18:23:43

Well, I got a little overzealous, I guess, because I just went ahead and wrote a little patch which solves the issue. I'm just going to commit and close this issue.
By: Digium Subversion (svnbot) 2008-10-16 18:24:06

Repository: asterisk
Revision: 150298

U   branches/1.4/main/manager.c

------------------------------------------------------------------------
r150298 | mmichelson | 2008-10-16 18:24:06 -0500 (Thu, 16 Oct 2008) | 10 lines

Don't try to call a dialplan function's read callback from
the manager's GetVar handler if an invalid channel has
been specified. Several dialplan functions, including
CHANNEL and SIP_HEADER, do not check for NULL-ness of
the channel being passed in.

(closes issue ASTERISK-12912)
Reported by: makoto


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

http://svn.digium.com/view/asterisk?view=rev&revision=150298
By: Mark Michelson (mmichelson) 2008-10-16 18:37:35

I'm reopening this...mainly because I'm an idiot. I didn't even think about the fact that not all dialplan functions actually require that a channel be defined. It makes much more sense to address the individual dialplan functions which actually make use of the channel without checking to be sure the channel is non-NULL.

So your patch for chan_sip.c will definitely help in this matter, but it seems like a good idea to tackle the other functions which may also be guilty of the same crime.
By: Mark Michelson (mmichelson) 2008-10-16 18:39:50

Updated the bug to indicate that this is a problem in 1.4.
By: Digium Subversion (svnbot) 2008-10-17 21:08:01

Repository: asterisk
Revision: 150817

_U  trunk/
U   trunk/main/manager.c

------------------------------------------------------------------------
r150817 | bweschke | 2008-10-17 21:08:00 -0500 (Fri, 17 Oct 2008) | 8 lines

Using the GetVar handler in AMI is potentially dangerous (insta-crash [tm]) when you use a dialplan function that requires a channel and then you don't provide one or provide an invalid one in the Channel: parameter. We'll handle this situation exactly the same way it was handled in pbx.c back on r61766.
We'll create a bogus channel for the function call and destroy it when we're done. If we have trouble allocating the bogus channel then we're not going to try executing the function call at all and run the risk of crashing.

(closes issue ASTERISK-12912)
reported by: makoto
patch by: bweschke


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

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