[FFmpeg-devel] [PATCH] Add a RTP depacketizer for the X-Qt format

[Ronald S. Bultje]

Hi,

On Tue, Oct 5, 2010 at 5:47 PM, Martin Storsj? <martin at martin.st> wrote:
> On Tue, 5 Oct 2010, Ronald S. Bultje wrote:
>> On Sun, Oct 3, 2010 at 11:45 AM, Martin Storsj? <martin at martin.st> wrote:
>> > This is the first brushed up version of the depacketizer for the X-Qt
>> > format (i.e., any quicktime supported format, in RTP), based on earlier
>> > work by Ronald.
>> [..]
>> > + ? ? ? ?if (qt->pkt.size > 0 && qt->timestamp == *timestamp) {
>> > +void* ptr = qt->pkt.data;
>> > + ? ? ? ? ? ?qt->pkt.data = av_realloc(qt->pkt.data, qt->pkt.size + alen + FF_INPUT_BUFFER_PADDING_SIZE);
>> > + ? ? ? ?} else {
>>
>> What is the void ptr doing there?
>
> It's stray debug code that I accidentally left there, I think I mentioned
> it on irc the other day that I had cleaned it up locally.
>
>> I think the code needs some length-checks, there's some possible
>> buffer overreads (and possible overwrites as a result of that in the
>> memcpy()s).
>
> I've done a lot of improvments regarding such things compared to your
> version that I started from - I'm quite confident with this version
> actually. It passes valgrind without any warnings or leaks.
[..]
> +    switch (packing_scheme) {
> +    case 3: { /* one data packet spread over 1 or multiple RTP packets */
> +        int alen = len - url_ftell(&pb);

At this point, alen can be negative which isn't checked in this part
of the code.

[..]
> +        memcpy(qt->pkt.data + qt->pkt.size, buf + url_ftell(&pb), alen);

And would then crash here.

Ronald