[Bug 774859] flic decoder: Invalid memory read in flx_decode_chunks

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=774859

--- Comment #5 from Matthew Waters (ystreet00) <ystreet00 at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #4)
> Review of attachment 340577 [details] [review]:
> 
> Generally looks good (just sanity-checked the parsing/writing). Just some
> comments that would be easy to fix, then merge please
> 
> ::: gst/flx/flx_color.c
> @@ -102,3 @@
>      memcpy (&flxpal->palvec[start * 3], newpal, grab * 3);
>    }
> -
> 
> There's a size calculation in this file: guint size = flxpal->width *
> flxpal->height
> 
> Do we check that this a) can't overflow and b) we have that much data
> available in the caller?

width/height are from the header and are at maximum 16-bit integers and
((2**16-1)**2) fits in 32-bits.

We always allocate width * height so yes, always available.

> ::: gst/flx/gstflxdec.c
> @@ +709,3 @@
> +  available = gst_adapter_available (flxdec->adapter);
> +  input = gst_adapter_get_buffer (flxdec->adapter, available);
> +  if (!gst_buffer_map (input, &map_info, GST_MAP_READ)) {
> 
> Just to be sure: get_buffer() is what is wanted here? It removes the whole
> data from the adapter, i.e. if it contains less than a complete frame, we
> would throw that away?

get_buffer() doesn't remove, take_buffer() does though.

> @@ +728,1 @@
>        gst_adapter_flush (flxdec->adapter, FlxHeaderSize);
> 
> You removed all above, so flushing here won't work

See above.

> @@ +747,2 @@
>        GST_LOG ("size      :  %d", flxh->size);
>        GST_LOG ("frames    :  %d", flxh->frames);
> 
> A bit further down, flxdec->size = ((guint) flxh->width * (guint)
> flxh->height)
> 
> And width/height come directly from the header. Do we make sure that a) this
> can't overflow (sanity check width/height) and later b) size * 4 can't
> (that's what we allocate)

size = width * height doesn't overflow (as above).

size * 4 will overflow with 32-bit gsize.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.