flacparse: Avoid integer overflow in available data check for image tags
authorSebastian Dröge <sebastian@centricular.com>
Tue, 13 Jun 2023 10:20:16 +0000 (13:20 +0300)
committerTim-Philipp Müller <tim@centricular.com>
Tue, 20 Jun 2023 08:16:37 +0000 (09:16 +0100)
If the image length as stored in the file is some bogus integer then
adding it to the current byte readers position can overflow and wrongly
have the check for enough available data succeed.

This then later can cause NULL pointer dereferences or out of bounds
reads/writes when actually reading the image data.

Fixes ZDI-CAN-20775
Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/2661

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4897>

subprojects/gst-plugins-good/gst/audioparsers/gstflacparse.c

index a53b7ebc776d653879e907d80d050a7793645c2d..8ee450c65ac0ac94c8260b62ed40e8a7236c0fe3 100644 (file)
@@ -1111,6 +1111,7 @@ gst_flac_parse_handle_picture (GstFlacParse * flacparse, GstBuffer * buffer)
   GstMapInfo map;
   guint32 img_len = 0, img_type = 0;
   guint32 img_mimetype_len = 0, img_description_len = 0;
+  const guint8 *img_data;
 
   gst_buffer_map (buffer, &map, GST_MAP_READ);
   gst_byte_reader_init (&reader, map.data, map.size);
@@ -1137,7 +1138,7 @@ gst_flac_parse_handle_picture (GstFlacParse * flacparse, GstBuffer * buffer)
   if (!gst_byte_reader_get_uint32_be (&reader, &img_len))
     goto error;
 
-  if (gst_byte_reader_get_pos (&reader) + img_len > map.size)
+  if (!gst_byte_reader_get_data (&reader, img_len, &img_data))
     goto error;
 
   GST_INFO_OBJECT (flacparse, "Got image of %d bytes", img_len);
@@ -1146,8 +1147,7 @@ gst_flac_parse_handle_picture (GstFlacParse * flacparse, GstBuffer * buffer)
     if (flacparse->tags == NULL)
       flacparse->tags = gst_tag_list_new_empty ();
 
-    gst_tag_list_add_id3_image (flacparse->tags,
-        map.data + gst_byte_reader_get_pos (&reader), img_len, img_type);
+    gst_tag_list_add_id3_image (flacparse->tags, img_data, img_len, img_type);
   }
 
   gst_buffer_unmap (buffer, &map);