meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.17k stars 2.47k forks source link

Missing NULL pointer checks for ice_handle->stream #1741

Closed tmatth closed 5 years ago

tmatth commented 5 years ago

Not easy to reproduce (only showed up during load testing):

ASAN:DEADLYSIGNAL
=================================================================
==13==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000388 (pc 0x55e17a603eea bp 0x7f28f42c45b0 sp 0x7f28f42c1a80 T7)
==13==The signal is caused by a WRITE memory access.
==13==Hint: address points to the zero page.
    #0 0x55e17a603ee9 in janus_plugin_handle_sdp /tmp/janus-gateway/janus.c:3279
    #1 0x55e17a5fd0c9 in janus_plugin_push_event /tmp/janus-gateway/janus.c:3064
...
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /tmp/janus-gateway/janus.c:3279 in janus_plugin_handle_sdp
Thread T7 created by T0 here:
    #0 0x7f2902c85d2f in __interceptor_pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x37d2f)
    #1 0x7f290230b42f  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x9242f)

==13==ABORTING

I believe this is because ice_handle->stream is sometimes used without checking if it's NULL. It is checked e.g., here https://github.com/meetecho/janus-gateway/blob/48b14a6640780cd93cac3c2891e123454671f5de/janus.c#L3327 just not everywhere.

zayim commented 5 years ago

Did you have this problem while using SIP plugin? I think I have the same issue, I managed to reproduce my by sending 200 OK response with SDP from B side to Janus, right after sending hangup from A side to Janus.

tmatth commented 5 years ago

Did you have this problem while using SIP plugin? I think I have the same issue, I managed to reproduce my by sending 200 OK response with SDP from B side to Janus, right after sending hangup from A side to Janus.

No in my case it was a custom plugin, but since it's in the core Janus code I imagine it could potentially happen with any plugin.