[Home]

Summary:ASTERISK-12110: segfault in command port of chan_ooh323
Reporter:Tzafrir Cohen (tzafrir)Labels:
Date Opened:2008-05-29 10:42:08Date Closed:2008-06-04 12:47:25
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Addons/chan_ooh323
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 12756.patch
( 1) fstab
( 2) ooh323_pipe.diff
( 3) ooh323_pipev2.diff
( 4) ooh323_pipev3.diff
Description:To reproduce this issue:

1. install chan_ooh323c with default configuration and load the module . tcp port 7575 will pop up.
2. write the standard HTTP text into it:

echo -e "GET / HTTP/1.0\n" nc localhost 7575

(by default it binds on port TCP 7575 on alll interfaces)

Result: Asterisk crashes.



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

Core was generated by `/usr/sbin/asterisk -f -U asterisk -G asterisk -vvvg -c'.
Program terminated with signal 11, Segmentation fault.
#0  0x00ded1dc in free () from /lib/libc.so.6
(gdb) bt
#0  0x00ded1dc in free () from /lib/libc.so.6
#1  0x0337c8c5 in ooReadAndProcessStackCommand ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
#2  0x0338b260 in ooProcessFDSETsAndTimers ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
#3  0x0338b53e in ooMonitorChannels ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
#4  0x0348a127 in ooh323c_stack_thread ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
ASTERISK-1  0x080fb82b in dummy_start (data=0x9a6b2f0) at utils.c:867
ASTERISK-2  0x0028d2da in start_thread () from /lib/libpthread.so.0
ASTERISK-3  0xb7e09480 in ?? ()
ASTERISK-4  0xb7e09480 in ?? ()
ASTERISK-5  0xb7e09480 in ?? ()
ASTERISK-6 0xb7e09480 in ?? ()
ASTERISK-7 0x00000000 in ?? ()
(gdb) bt full
#0  0x00ded1dc in free () from /lib/libc.so.6
No symbol table info available.
#1  0x0337c8c5 in ooReadAndProcessStackCommand ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
No symbol table info available.
#2  0x0338b260 in ooProcessFDSETsAndTimers ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
No symbol table info available.
#3  0x0338b53e in ooMonitorChannels ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
No symbol table info available.
#4  0x0348a127 in ooh323c_stack_thread ()
  from /usr/lib/asterisk/modules/chan_ooh323.so
No symbol table info available.
ASTERISK-1  0x080fb82b in dummy_start (data=0x9a6b2f0) at utils.c:867
       __cancel_buf = {__cancel_jmp_buf = {{__cancel_jmp_buf = {161921264, 0,
       -1210016880, -1210018872, -451260560, 1526031380},
     __mask_was_saved = 0}}, __pad = {0xb7e09480, 0x0, 0x0, 0x0}}
       __cancel_arg = (void *) 0xb7e09b90
       not_first_call = <value optimized out>
       ret = <value optimized out>
ASTERISK-2  0x0028d2da in start_thread () from /lib/libpthread.so.0
No symbol table info available.
ASTERISK-3  0xb7e09480 in ?? ()
No symbol table info available.
ASTERISK-4  0xb7e09480 in ?? ()
No symbol table info available.
ASTERISK-5  0xb7e09480 in ?? ()
No symbol table info available.
ASTERISK-6 0xb7e09480 in ?? ()
No symbol table info available.
ASTERISK-7 0x00000000 in ?? ()
No symbol table info available.
Comments:By: Mark Michelson (mmichelson) 2008-05-29 15:29:07

Hi Tzafrir, I've got a patch for you try out. The patch consists of two parts.

First, there's a command socket that appeared to be used for internal communication. This socket was bound to port 7575 and was where you were able to cause the crash. I changed this socket to be bound to localhost instead of the IP address set in ooh323.conf. This prevents the crash from being exploited remotely.

Secondly, I was able to stop local crashes (and 100% CPU loops as you also found) by adding error-checking to ooProcessFDSETsAndTimers.

If you can still cause crashes after applying the patch, let me know.
By: Tzafrir Cohen (tzafrir) 2008-05-29 16:58:15

Getting closer. My fstab can still crash it:

cat /etc/fstab | nc -q 0 localhost 7575
By: Mark Michelson (mmichelson) 2008-05-29 17:40:14

Thanks for providing the fstab. I ran your test with your fstab and found the cause of the crash. It is because the function where the crash is occurring reads addresses from a socket and then frees the memory at those addresses. The problem is that if you don't pass valid pointers, then the function attempts to free bogus memory. Essentially this is what you're doing by copying a bunch of random text to the socket.

The best way I can think of to both foil this crash and not potentially leak memory would be to somehow ensure that the data read from the socket was generated by ooh323 and not passed in from elsewhere. If the data was passed in from somewhere else, then we should not try to process it at all and just return without attempting to free any memory.
By: Mark Michelson (mmichelson) 2008-05-30 16:06:51

I've written a new patch which attempts to change the communication method used within ooh323 from a TCP socket to a pipe instead.

The problem I have right now is that I have no way to test this. I have no H.323 phones. So far this patch passes the compilation test, and Asterisk seems to run just fine with chan_ooh323.so loaded, but until calls can be passed through, I won't know if I've totally broken this. Do you have any way that you can apply this patch and place some H.323 calls through Asterisk?

I have a feeling that I may need to add some locking around the reading and writing of the pipe to ensure that the data is ordered properly, but for now I just want to be sure that a small-scale basic test will not completely blow Asterisk up.
By: Mark Michelson (mmichelson) 2008-05-30 17:08:06

Uploaded v2. chan_ooh323.so was not loading correctly, and this fixed that.
By: Tzafrir Cohen (tzafrir) 2008-06-02 07:34:57

Isn't a pipe intended for 1-1 communication? A socket will have a new pair talking on each connection.
By: Mark Michelson (mmichelson) 2008-06-02 10:06:06

What you say about TCP connections holds true in general, but in the case of ooh323's stack command connection, only a single connection was established and this connection was reused with each subsequent command which was issued. Because only one connection existed at any given time, it is very easy to translate the model to use a pipe instead.
By: Mark Michelson (mmichelson) 2008-06-02 16:35:37

I ran a quick test where I placed a call over H.323 between Asterisk boxes and the stack command communication appeared to be working properly on a basic level using a pipe. So, there's at least some degree of success to report here.
By: Digium Subversion (svnbot) 2008-06-04 11:49:22

Repository: asterisk-addons
Revision: 620

U   branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.c
U   branches/1.2/asterisk-ooh323c/ooh323c/src/ooCmdChannel.h
U   branches/1.2/asterisk-ooh323c/ooh323c/src/oochannels.c
U   branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.c
U   branches/1.2/asterisk-ooh323c/ooh323c/src/ooh323ep.h
U   branches/1.2/asterisk-ooh323c/src/chan_h323.c

------------------------------------------------------------------------
r620 | mmichelson | 2008-06-04 11:49:21 -0500 (Wed, 04 Jun 2008) | 15 lines

A few changes:

1. Add error-checking to the call to ooReadAndProcessStackCommands in oochannels.c
2. (1.2 only) There was a char * being used before being set. This was causing an almost
  immediate crash when ooh323 would load. This commit fixes that issue.
3. Most importantly, fix a security issue wherein arbitrary data could be sent to ooh323's
  command stack listening socket causing a remote crash. See AST-2008-009 for more details.

(closes issue ASTERISK-12110)
Reported by: tzafrir
Patches:
     ooh323_pipev3.diff uploaded by putnopvut (license 60)
Tested by: putnopvut


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

http://svn.digium.com/view/asterisk-addons?view=rev&revision=620
By: Digium Subversion (svnbot) 2008-06-04 11:51:21

Repository: asterisk-addons
Revision: 621

U   branches/1.4/channels/chan_ooh323.c
U   branches/1.4/channels/ooh323c/src/ooCmdChannel.c
U   branches/1.4/channels/ooh323c/src/ooCmdChannel.h
U   branches/1.4/channels/ooh323c/src/oochannels.c
U   branches/1.4/channels/ooh323c/src/ooh323ep.c
U   branches/1.4/channels/ooh323c/src/ooh323ep.h

------------------------------------------------------------------------
r621 | mmichelson | 2008-06-04 11:51:21 -0500 (Wed, 04 Jun 2008) | 20 lines

------------------------------------------------------------------------
r620 | mmichelson | 2008-06-04 11:55:58 -0500 (Wed, 04 Jun 2008) | 15 lines

A few changes:

1. Add error-checking to the call to ooReadAndProcessStackCommands in oochannels.c
2. (1.2 only) There was a char * being used before being set. This was causing an almost
  immediate crash when ooh323 would load. This commit fixes that issue.
3. Most importantly, fix a security issue wherein arbitrary data could be sent to ooh323's
  command stack listening socket causing a remote crash. See AST-2008-009 for more details.

(closes issue ASTERISK-12110)
Reported by: tzafrir
Patches:
     ooh323_pipev3.diff uploaded by putnopvut (license 60)
Tested by: putnopvut


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

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

http://svn.digium.com/view/asterisk-addons?view=rev&revision=621
By: Digium Subversion (svnbot) 2008-06-04 12:28:50

Repository: asterisk-addons
Revision: 627

U   trunk/channels/chan_ooh323.c
U   trunk/channels/ooh323c/src/ooCapability.c
U   trunk/channels/ooh323c/src/ooCmdChannel.c
U   trunk/channels/ooh323c/src/ooCmdChannel.h
U   trunk/channels/ooh323c/src/ooSocket.c
U   trunk/channels/ooh323c/src/oochannels.c
U   trunk/channels/ooh323c/src/ooh323ep.c
U   trunk/channels/ooh323c/src/ooh323ep.h
U   trunk/channels/ooh323c/src/ootrace.c
U   trunk/channels/ooh323c/src/ootrace.h

------------------------------------------------------------------------
r627 | mmichelson | 2008-06-04 12:28:49 -0500 (Wed, 04 Jun 2008) | 20 lines

------------------------------------------------------------------------
r620 | mmichelson | 2008-06-04 11:55:58 -0500 (Wed, 04 Jun 2008) | 15 lines

A few changes:

1. Add error-checking to the call to ooReadAndProcessStackCommands in oochannels.c
2. (1.2 only) There was a char * being used before being set. This was causing an almost
  immediate crash when ooh323 would load. This commit fixes that issue.
3. Most importantly, fix a security issue wherein arbitrary data could be sent to ooh323's
  command stack listening socket causing a remote crash. See AST-2008-009 for more details.

(closes issue ASTERISK-12110)
Reported by: tzafrir
Patches:
     ooh323_pipev3.diff uploaded by putnopvut (license 60)
Tested by: putnopvut


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

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

http://svn.digium.com/view/asterisk-addons?view=rev&revision=627
By: Digium Subversion (svnbot) 2008-06-04 12:47:25

Repository: asterisk-addons
Revision: 628

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_ooh323.c
U   branches/1.6.0/channels/ooh323c/src/ooCapability.c
U   branches/1.6.0/channels/ooh323c/src/ooCmdChannel.c
U   branches/1.6.0/channels/ooh323c/src/ooCmdChannel.h
U   branches/1.6.0/channels/ooh323c/src/ooSocket.c
U   branches/1.6.0/channels/ooh323c/src/oochannels.c
U   branches/1.6.0/channels/ooh323c/src/ooh323ep.c
U   branches/1.6.0/channels/ooh323c/src/ooh323ep.h
U   branches/1.6.0/channels/ooh323c/src/ootrace.c
U   branches/1.6.0/channels/ooh323c/src/ootrace.h

------------------------------------------------------------------------
r628 | mmichelson | 2008-06-04 12:47:24 -0500 (Wed, 04 Jun 2008) | 28 lines

Merged revisions 627 via svnmerge from
https://origsvn.digium.com/svn/asterisk-addons/trunk

........
r627 | mmichelson | 2008-06-04 12:35:29 -0500 (Wed, 04 Jun 2008) | 20 lines

------------------------------------------------------------------------
r620 | mmichelson | 2008-06-04 11:55:58 -0500 (Wed, 04 Jun 2008) | 15 lines

A few changes:

1. Add error-checking to the call to ooReadAndProcessStackCommands in oochannels.c
2. (1.2 only) There was a char * being used before being set. This was causing an almost
  immediate crash when ooh323 would load. This commit fixes that issue.
3. Most importantly, fix a security issue wherein arbitrary data could be sent to ooh323's
  command stack listening socket causing a remote crash. See AST-2008-009 for more details.

(closes issue ASTERISK-12110)
Reported by: tzafrir
Patches:
     ooh323_pipev3.diff uploaded by putnopvut (license 60)
Tested by: putnopvut


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

........

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

http://svn.digium.com/view/asterisk-addons?view=rev&revision=628