[rtmpdump] Null dereference in rtmpsuck

[Matt Breedlove]

Howard,

I appreciate the response.  With that in mind and looking at the code a
little more, it seems like the two RTMP_IsConnected calls for each of the
streams and setting the FDs for the select would trigger the check below
that causes the crash provided server->rs is still valid while server->rc
is in a closed state.  I don't see anything in the code below the
RMTP_IsConnected calls that would verify that server->rc is even open at
the time of the select since select is called as long as one of the two are
open.  It looks like the problem is the assumption that server->rc is even
still valid at the time the select goes through.

Anyway, I'm going to take you up on your suggestion and test with it since
it seems preferable to follow the standard instead of using a one-off.  How
would this generally make it into the git repository?  Documentation is a
bit lacking in regards to that.

Thanks again



On Mon, Oct 27, 2014 at 10:16 AM, Howard Chu <hyc at highlandsun.com> wrote:

> 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.
>
> _______________________________________________
> rtmpdump mailing list
> rtmpdump at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/rtmpdump
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.mplayerhq.hu/pipermail/rtmpdump/attachments/20141027/e43044ed/attachment.html>