[rtmpdump] Null dereference in rtmpsuck

[Howard Chu]

Matt Breedlove wrote:
> Hey guys,
>
> I've run into a case that happens while running rtmpsuck where a null
> dereference will eventually occur.  I'm running a Windows build currently but
> this was present in the Linux build also from the latest version on back.
>
>
> FAULTING_IP:
> image00400000+2eab
> 00402eab 8b0482          mov     eax,dword ptr [edx+eax*4]
>
> EXCEPTION_RECORD:  ffffffff -- (.exr 0xffffffffffffffff)
> ExceptionAddress: 00402eab (image00400000+0x00002eab)
>     ExceptionCode: c0000005 (Access violation)
>    ExceptionFlags: 00000000
> NumberParameters: 2
>     Parameter[0]: 00000000
>     Parameter[1]: 00000018
> Attempt to read from address 00000018
>
> CONTEXT:  00000000 -- (.cxr 0x0;r)
> eax=00000006 ebx=0b835be0 ecx=0dce1298 edx=00000000 esi=0e60fdb8 edi=0b835bf0
> eip=00402eab esp=0e60fd3c ebp=0e60ff14 iopl=0         nv up ei pl zr na pe nc
> cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
> image00400000+0x2eab:
> 00402eab 8b0482          mov     eax,dword ptr [edx+eax*4]
> ds:002b:00000018=????????
>
>
> rtmpsuck.c on line 818:
>
> server->rc.m_pauseStamp =
> server->rc.m_channelTimestamp[server->rc.m_mediaChannel];
>
> compiles to:
>
> movecx, [ebx+8418h]
> testecx, ecx
> jzshort loc_402EE2# jz server->f_cur
> moveax, [ebx+4224h]# server->rc.m_mediaChannel => eax (6)
> testeax, eax
> jzshort loc_402EE2# jz server->rc.m_mediaChannel
> movedx, [ebp+var_1A4]
> testedx, edx
> jnzshort loc_402EE2# jnz paused
> movedx, [ebx+425Ch]# server->rc.m_channelTimestamp => edx (0)
> moveax, [edx+eax*4]# edx(0)+eax(6)*4 == 0x18
> mov[ebx+422Ch], eax
> moveax, [ebp+var_19C]
> mov[esp], eax
> call_RTMP_ToggleStream
> testeax, eax
>
>
> The following patch seems to at least prevent the process from crashing but I
> was hoping to get some input in case this might just be a symptom of another
> problem.  I'm not familiar enough with the protocol or codebase.  Any thoughts?
>
> rtmpsuck.c:
>
> 818c818
> <               if (server->f_cur && server->rc.m_mediaChannel &&
> server->rc.m_channelTimestamp && !paused)
> ---
>  >               if (server->f_cur && server->rc.m_mediaChannel && !paused)
>
>
> Regards,
> Matt Breedlove

The only way for m_channelTimestamp to be NULL is if RTMP_Close was called on 
that handle. Your patch will work fine, though it may not be the most obvious 
check. Typically you'd check for m_stream_id >= 0 to see if the handle is 
still alive.