After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775070 - asfdemux: ASF object size checks
asfdemux: ASF object size checks
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
unspecified
Other All
: Normal normal
: 1.10.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-25 08:46 UTC by Edward Hervey
Modified: 2016-11-25 11:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
asfdemux: Add check for invalid/corrupt asf object (9.33 KB, patch)
2016-11-25 08:46 UTC, Edward Hervey
none Details | Review
asfdemux: Add sanity checks when reading asf_stream_video_format (2.08 KB, patch)
2016-11-25 08:46 UTC, Edward Hervey
none Details | Review
asfdemux: Add check for invalid/corrupt asf object (9.33 KB, patch)
2016-11-25 09:08 UTC, Edward Hervey
committed Details | Review
asfdemux: Add sanity checks when reading asf_stream_video_format (2.08 KB, patch)
2016-11-25 09:08 UTC, Edward Hervey
committed Details | Review
asfdemux: Add sanity check for asf_stream_audio (1.03 KB, patch)
2016-11-25 09:08 UTC, Edward Hervey
committed Details | Review

Description Edward Hervey 2016-11-25 08:46:10 UTC
Add checks for ASF object size
Comment 1 Edward Hervey 2016-11-25 08:46:15 UTC
Created attachment 340733 [details] [review]
asfdemux: Add check for invalid/corrupt asf object

An asf object can't realistically be bigger than 2**32 bytes.

If it reports a size bigger than that, consider it corrupt and properly
propagate the error back.
Comment 2 Edward Hervey 2016-11-25 08:46:21 UTC
Created attachment 340734 [details] [review]
asfdemux: Add sanity checks when reading asf_stream_video_format

It should report a size of at least 40 bytes
Also check for the size of the remaining data (i.e. codec_data)
Comment 3 Edward Hervey 2016-11-25 09:08:37 UTC
Created attachment 340735 [details] [review]
asfdemux: Add check for invalid/corrupt asf object

An asf object can't realistically be bigger than 2**32 bytes.

If it reports a size bigger than that, consider it corrupt and properly
propagate the error back.
Comment 4 Edward Hervey 2016-11-25 09:08:44 UTC
Created attachment 340736 [details] [review]
asfdemux: Add sanity checks when reading asf_stream_video_format

It should report a size of at least 40 bytes
Also check for the size of the remaining data (i.e. codec_data)
Comment 5 Edward Hervey 2016-11-25 09:08:52 UTC
Created attachment 340737 [details] [review]
asfdemux: Add sanity check for asf_stream_audio

We should have enough bytes for the specified codec_data
Comment 6 Sebastian Dröge (slomo) 2016-11-25 10:52:12 UTC
Review of attachment 340735 [details] [review]:

Looks good

::: gst/asfdemux/gstasfdemux.c
@@ +1119,3 @@
     bufdata = (guint8 *) map.data;
+    obj_size = obj.size;
+    ret = gst_asf_demux_process_object (demux, &bufdata, &obj_size);

Why don't you continue passing obj.size (and change all the things to guint32*)?
Comment 7 Edward Hervey 2016-11-25 11:04:06 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)
> Why don't you continue passing obj.size (and change all the things to
> guint32*)?

Channging all the things to guint32 would have required ... well... changing *really* all the things to guint32 (i.e. it was way way way too intrusive).
Comment 8 Edward Hervey 2016-11-25 11:05:06 UTC
commit 4a52e6c827b30dc34fb177c6362ce02f1068e718
Author: Edward Hervey <edward@centricular.com>
Date:   Fri Nov 25 10:07:35 2016 +0100

    asfdemux: Add sanity check for asf_stream_audio
    
    We should have enough bytes for the specified codec_data
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775070

commit 8f05e8f44979c84aaf06f0c817e76eb44b28de50
Author: Edward Hervey <edward@centricular.com>
Date:   Fri Nov 25 09:45:04 2016 +0100

    asfdemux: Add sanity checks when reading asf_stream_video_format
    
    It should report a size of at least 40 bytes
    Also check for the size of the remaining data (i.e. codec_data)
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775070

commit 97294eb8bbed1b9dad7d3f2c52dd69eb1812cc06
Author: Edward Hervey <edward@centricular.com>
Date:   Fri Nov 25 09:44:05 2016 +0100

    asfdemux: Add check for invalid/corrupt asf object
    
    An asf object can't realistically be bigger than 2**32 bytes.
    
    If it reports a size bigger than that, consider it corrupt and properly
    propagate the error back.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775070
Comment 9 Edward Hervey 2016-11-25 11:07:36 UTC
And also pushed to 1.10