xorl %eax, %eax

CVE-2009-1932: GStreamer Plug-ins Integer Overflows

leave a comment »

This issue was reported by Tielei Wang and and finally fixed by Jan Schmidt This CVE ID includes four similar integer overflow vulnerabilities located at ext/libpng/gstpngdec.c. GStreamer is a popular library for numerous different media formats and it is installed by default in major Linux distributions such as Madrake Linux. The buggy code is part of the PNG decoding routines of GStreamer, here is the vulnerable code from 0.10.15 release of gst-plugins-good which is the last vulnerable version.

static void
user_info_callback (png_structp png_ptr, png_infop info)
{
 GstPngDec *pngdec = NULL;
 GstFlowReturn ret = GST_FLOW_OK;
 size_t buffer_size;
 GstBuffer *buffer = NULL;
 ...
 /* Allocate output buffer */
 pngdec->rowbytes = png_get_rowbytes (pngdec->png, pngdec->info);
 buffer_size = pngdec->height * GST_ROUND_UP_4 (pngdec->rowbytes);
 ret =
 gst_pad_alloc_buffer_and_set_caps (pngdec->srcpad, GST_BUFFER_OFFSET_NONE, buffer_size, GST_PAD_CAPS (pngdec->srcpad), &buffer);
 if (ret != GST_FLOW_OK) {
 goto beach;
 }

 pngdec->buffer_out = buffer;

beach:
 pngdec->ret = ret;
}

The bug is present at the calculation of the size to be allocated (buffer_size). It retrieves the rowbytes using png_get_rowbytes() and then uses this value multiplied by the image’s height to calculate the buffer size. Clearly this can result in integer overflow and consequently an invalid allocation using gst_pad_alloc_buffer_and_set_caps(). To fix this they applied the following patch:

 pngdec->rowbytes = png_get_rowbytes (pngdec->png, pngdec->info);
-  buffer_size = pngdec->height * GST_ROUND_UP_4 (pngdec->rowbytes);
+  if (pngdec->rowbytes > (G_MAXUINT32 - 3)
+      || pngdec->height > G_MAXUINT32 / pngdec->rowbytes) {
+    ret = GST_FLOW_ERROR;
+    goto beach;
+  }
+  pngdec->rowbytes = GST_ROUND_UP_4 (pngdec->rowbytes);
+  buffer_size = pngdec->height * pngdec->rowbytes;
+
 ret =

This way it checks that rowbytes cannot be larger than G_MAXUINT32 – 3 and PNG image’s height cannot be more than G_MAXUINT32 / rowbytes. The next integer overflow vulnerability can be found a few lines later at user_endrow_callback() function like this:

static void
user_endrow_callback (png_structp png_ptr, png_bytep new_row,
 png_uint_32 row_num, int pass)
{
 GstPngDec *pngdec = NULL;

 pngdec = GST_PNGDEC (png_ptr->io_ptr);

 /* FIXME: implement interlaced pictures */

 /* If buffer_out doesn't exist, it means buffer_alloc failed, which
 * will already have set the return code */
 if (GST_IS_BUFFER (pngdec->buffer_out)) {
 size_t offset = row_num * GST_ROUND_UP_4 (pngdec->rowbytes);

 GST_LOG ("got row %u, copying in buffer %p at offset %" G_GSIZE_FORMAT,
 (guint) row_num, pngdec->buffer_out, offset);
 memcpy (GST_BUFFER_DATA (pngdec->buffer_out) + offset, new_row,
 pngdec->rowbytes);
 pngdec->ret = GST_FLOW_OK;
 }
}

As you can see, during the offset calculation a similar integer overflow takes place. The offset is used to copy the new row at the end using memcpy() and to calculate it, it uses: row_number * rowbytes. However, it utilizes GST_ROUND_UP_4() macro which is used to round the passed integer to the next multiple of 4. This might lead to integer overflow. To fix it, they simply removed this macro like this:

 if (GST_IS_BUFFER (pngdec->buffer_out)) {
-    size_t offset = row_num * GST_ROUND_UP_4 (pngdec->rowbytes);
+    size_t offset = row_num * pngdec->rowbytes;

Next, the buggy routine is:

static void
gst_pngdec_task (GstPad * pad)
{
 GstPngDec *pngdec;
 GstBuffer *buffer = NULL;
 size_t buffer_size = 0;
 gint i = 0;
 png_bytep *rows, inp;
 png_uint_32 rowbytes;
 GstFlowReturn ret = GST_FLOW_OK;
 ...
 /* Allocate output buffer */
 rowbytes = png_get_rowbytes (pngdec->png, pngdec->info);
 buffer_size = pngdec->height * GST_ROUND_UP_4 (rowbytes);
 ret =
 gst_pad_alloc_buffer_and_set_caps (pngdec->srcpad, GST_BUFFER_OFFSET_NONE,
 buffer_size, GST_PAD_CAPS (pngdec->srcpad), &buffer);
 if (ret != GST_FLOW_OK)
 goto pause;
 ...
}

This is exactly the same as the first one. The patch was of course:

 rowbytes = png_get_rowbytes (pngdec->png, pngdec->info);
-  buffer_size = pngdec->height * GST_ROUND_UP_4 (rowbytes);
+  if (rowbytes > (G_MAXUINT32 - 3) || pngdec->height > G_MAXUINT32 / rowbytes) {
+    ret = GST_FLOW_ERROR;
+    goto pause;
+  }
+  rowbytes = GST_ROUND_UP_4 (rowbytes);
+  buffer_size = pngdec->height * rowbytes;
 ret =
 gst_pad_alloc_buffer_and_set_caps (pngdec->srcpad, GST_BUFFER_OFFSET_NONE,
 buffer_size, GST_PAD_CAPS (pngdec->srcpad), &buffer);

The last one is located at the same function like this:

static void
gst_pngdec_task (GstPad * pad)
{
 GstPngDec *pngdec;
 GstBuffer *buffer = NULL;
 size_t buffer_size = 0;
 gint i = 0;
 png_bytep *rows, inp;
 png_uint_32 rowbytes;
 GstFlowReturn ret = GST_FLOW_OK;
 ...
 inp = GST_BUFFER_DATA (buffer);

 for (i = 0; i < pngdec->height; i++) {
 rows[i] = inp;
 inp += GST_ROUND_UP_4 (rowbytes);
 }
 ...
}

Here it attempts to iterate through each row and it stores them into rows[] array. Pointer inp is incremented by the equivalent rowbytes on each iteration but GST_ROUND_UP_4() may result in integer overflows. The fix was simple:

 rows[i] = inp;
-    inp += GST_ROUND_UP_4 (rowbytes);
+    inp += rowbytes;
 }

Written by xorl

June 6, 2009 at 19:05

Posted in bugs

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s