[Home]

Summary:ASTERISK-05956: Remote crash when sending special BYE/Also packet
Reporter:Guenther Starnberger (gst)Labels:
Date Opened:2006-01-02 14:02:45.000-0600Date Closed:2006-01-24 13:00:28.000-0600
Priority:CriticalRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip2.patch
Description:Sending the following SIP packet to Asterisk causes a segfault:

---------- 8< ---------- 8< ---------- 8< ---------- 8< ----------
BYE sip:dummy@11.22.33.44 SIP/2.0
From: sip:dummay@12.34.56.78;user=phone
To: sip:dummy@12.34.56.78:5060;user=phone
Contact: sip:dummy@12.34.56.78;user=phone
CSeq: 100 BYE
Call-ID: 9787963-3319555348-76751@100.240.90.99
Max-Forwards: 16
Content-Length: 0
Also: foo
---------- 8< ---------- 8< ---------- 8< ---------- 8< ----------

Save in a textfile foo.txt and use "sipsak -f foo.txt -s sip:some.server" to crash a server.

I haven't debugged this further but it seems that the existence of p->owner isn't checked when doing an 'Also' transfer. This may be the cause of the problem.


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

Disclaimer: I have only tested this against beta2 and 1.2.1. Currently I have no time to test this against other versions. I am partly offline during this week so replies may take longer.
Comments:By: Tilghman Lesher (tilghman) 2006-01-02 15:05:59.000-0600

Please upload a backtrace, as specified in the bug reporting guidelines.
By: Peng Yong (ppyy) 2006-01-02 19:53:02.000-0600

sorry, there are two patches (another is code clean for asterisk.c) in  first upload file. second upload is correct:

not check p->owner before ast_queue_hangup(p->owner)
By: Clod Patry (junky) 2006-01-02 20:50:25.000-0600

ppyy: do you have disclaimer on file for that patch?
By: Peng Yong (ppyy) 2006-01-03 01:52:46.000-0600

disclaimer on file: yes
By: Peng Yong (ppyy) 2006-01-03 05:34:02.000-0600

i found many sources have following lines:

if (chann)
ast_queue_hangup(chann);

if one developer forgot one if(chan), it may cause crash.

i think we should patch ast_queue_hangup and remove all these if(chan):

Index: channel.c
===================================================================
--- channel.c   (revision 7717)
+++ channel.c   (working copy)
@@ -655,6 +655,10 @@
int ast_queue_hangup(struct ast_channel *chan)
{
       struct ast_frame f = { AST_FRAME_CONTROL, AST_CONTROL_HANGUP };
+
+       if (!chan)
+               return -1;
+
       /* Yeah, let's not change a lock-critical value without locking */
       if (!ast_mutex_trylock(&chan->lock)) {
               chan->_softhangup |= AST_SOFTHANGUP_DEV;


By: Olle Johansson (oej) 2006-01-24 12:25:57.000-0600

Fixed in 1.2 and svn trunk. Thanks!