bytereader: fix peek value when scanning for 00 00 01 with non-0 offset

We would add the offset a second time in _scan_for_start_code()
when we found a result, but it's already been added to the data
pointer at the beginning of _masked_scan_uint32_peek(), so the
peeked value would be wrong if the initial offset was >0, and
we would potentially read memory out-of-bounds.

Add unit test for all of this.

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

2 files changed, 50 insertions, 5 deletions

diff --git a/libs/gst/base/gstbytereader.c b/libs/gst/base/gstbytereader.c
index baa152fd1..0045ce93e 100644
--- a/libs/gst/base/gstbytereader.c
+++ b/libs/gst/base/gstbytereader.c
@@ -822,7 +822,7 @@ gst_byte_reader_dup_data (GstByteReader * reader, guint size, guint8 ** val)
 
 /* Special optimized scan for mask 0xffffff00 and pattern 0x00000100 */
 static inline gint
-_scan_for_start_code (const guint8 * data, guint offset, guint size)
+_scan_for_start_code (const guint8 * data, guint size)
 {
   guint8 *pdata = (guint8 *) data;
   guint8 *pend = (guint8 *) (data + size - 4);
@@ -835,7 +835,7 @@ _scan_for_start_code (const guint8 * data, guint offset, guint size)
     } else if (pdata[0] || pdata[2] != 1) {
       pdata++;
     } else {
-      return (pdata - data + offset);
+      return (pdata - data);
     }
   }
 
@@ -863,10 +863,15 @@ _masked_scan_uint32_peek (const GstByteReader * reader,
 
   /* Handle special case found in MPEG and H264 */
   if ((pattern == 0x00000100) && (mask == 0xffffff00)) {
-    guint ret = _scan_for_start_code (data, offset, size);
-    if (G_UNLIKELY (value))
+    gint ret = _scan_for_start_code (data, size);
+
+    if (ret == -1)
+      return ret;
+
+    if (value != NULL)
       *value = (1 << 8) | data[ret + 3];
-    return ret;
+
+    return ret + offset;
   }
 
   /* set the state to something that does not match */
diff --git a/tests/check/libs/bytereader.c b/tests/check/libs/bytereader.c
index fd4f95dc0..24a29714d 100644
--- a/tests/check/libs/bytereader.c
+++ b/tests/check/libs/bytereader.c
@@ -563,6 +563,46 @@ GST_START_TEST (test_scan)
 
   /* not enough bytes */
   do_scan (&reader, 0xffffffff, 0xc4c5c6c7, 0x44, 99, -1);
+
+  /* check special code path that exists for 00 00 01 sync marker */
+  {
+    const guint8 sync_data[] = { 0xA0, 0x00, 0x00, 0x00, 0x01, 0xA5, 0xA6,
+      0xA7, 0xA8, 0xA9, 0xAA, 0x00, 0x00, 0x00, 0x01, 0xAF, 0xB0, 0xB1
+    };
+    guint32 val = 0;
+    guint8 *m;
+    gint found;
+
+    /* dup so valgrind can detect out of bounds access more easily */
+    m = g_memdup (sync_data, sizeof (sync_data));
+    gst_byte_reader_init (&reader, m, sizeof (sync_data));
+
+    found = gst_byte_reader_masked_scan_uint32_peek (&reader, 0xffffff00,
+        0x00000100, 0, sizeof (sync_data), &val);
+    fail_unless_equals_int (found, 2);
+    fail_unless_equals_int (val, 0x000001A5);
+
+    found = gst_byte_reader_masked_scan_uint32_peek (&reader, 0xffffff00,
+        0x00000100, 2, sizeof (sync_data) - 2, &val);
+    fail_unless_equals_int (found, 2);
+    fail_unless_equals_int (val, 0x000001A5);
+
+    found = gst_byte_reader_masked_scan_uint32_peek (&reader, 0xffffff00,
+        0x00000100, 3, sizeof (sync_data) - 3, &val);
+    fail_unless_equals_int (found, 12);
+    fail_unless_equals_int (val, 0x000001AF);
+
+    found = gst_byte_reader_masked_scan_uint32_peek (&reader, 0xffffff00,
+        0x00000100, 12, sizeof (sync_data) - 12, &val);
+    fail_unless_equals_int (found, 12);
+    fail_unless_equals_int (val, 0x000001AF);
+
+    found = gst_byte_reader_masked_scan_uint32_peek (&reader, 0xffffff00,
+        0x00000100, 13, sizeof (sync_data) - 13, &val);
+    fail_unless_equals_int (found, -1);
+
+    g_free (m);
+  }
 }
 
 GST_END_TEST;