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

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #340577|none                        |needs-work
             status|                            |

--- Comment #4 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 340577:
 --> (https://bugzilla.gnome.org/review?bug=774859&attachment=340577)

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?

::: 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?

@@ +728,1 @@
       gst_adapter_flush (flxdec->adapter, FlxHeaderSize);

You removed all above, so flushing here won't work

@@ +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)

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