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

[Martin Storsjö]

On Tue, 5 Oct 2010, Ronald S. Bultje wrote:

> 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.

Good catch. Checking if (alen <= 0) as the other switch case already does.

// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-RTP-depacketization-of-the-X-QT-QuickTime-format.patch
Type: text/x-diff
Size: 13271 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101006/f797550b/attachment.patch>