h2645_parse: use the bytestream2 API for packet splitting

The code does some nontrivial jumping around in the buffer, so it is
safer to use a checked API rather than do everything manually.

Fixes a bug in nalff parsing, where the length field is currently not
counted in the buffer size check, resulting in possible overreads with
invalid files.

CC: libav-stable@libav.org
Bug-Id: 1002
Found-By: Kamil Frankowicz

1 files changed, 21 insertions, 22 deletions

diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
index 8492425..b507b19 100644
--- a/libavcodec/h2645_parse.c
+++ b/libavcodec/h2645_parse.c
@@ -26,6 +26,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/mem.h"
 
+#include "bytestream.h"
 #include "h2645_parse.h"
 
 int ff_h2645_extract_rbsp(const uint8_t *src, int length,
@@ -214,11 +215,14 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length,
                           void *logctx, int is_nalff, int nal_length_size,
                           enum AVCodecID codec_id)
 {
+    GetByteContext bc;
     int consumed, ret = 0;
-    const uint8_t *next_avc = buf + (is_nalff ? 0 : length);
+    size_t next_avc = is_nalff ? 0 : length;
+
+    bytestream2_init(&bc, buf, length);
 
     pkt->nb_nals = 0;
-    while (length >= 4) {
+    while (bytestream2_get_bytes_left(&bc) >= 4) {
         H2645NAL *nal;
         int extract_length = 0;
         int skip_trailing_zeros = 1;
@@ -231,40 +235,37 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length,
          * correctly and consumes only the first NAL unit. The additional NAL
          * units are handled here in the Annex B parsing code.
          */
-        if (buf == next_avc) {
+        if (bytestream2_tell(&bc) == next_avc) {
             int i;
             for (i = 0; i < nal_length_size; i++)
-                extract_length = (extract_length << 8) | buf[i];
+                extract_length = (extract_length << 8) | bytestream2_get_byte(&bc);
 
-            if (extract_length > length) {
+            if (extract_length > bytestream2_get_bytes_left(&bc)) {
                 av_log(logctx, AV_LOG_ERROR,
                        "Invalid NAL unit size (%d > %d).\n",
-                       extract_length, length);
+                       extract_length, bytestream2_get_bytes_left(&bc));
                 return AVERROR_INVALIDDATA;
             }
-            buf     += nal_length_size;
-            length  -= nal_length_size;
             // keep track of the next AVC1 length field
-            next_avc = buf + extract_length;
+            next_avc = bytestream2_tell(&bc) + extract_length;
         } else {
             /*
              * expected to return immediately except for streams with mixed
              * NAL unit coding
              */
-            int buf_index = find_next_start_code(buf, next_avc);
+            int buf_index = find_next_start_code(bc.buffer, buf + next_avc);
 
-            buf    += buf_index;
-            length -= buf_index;
+            bytestream2_skip(&bc, buf_index);
 
             /*
              * break if an AVC1 length field is expected at the current buffer
              * position
              */
-            if (buf == next_avc)
+            if (bytestream2_tell(&bc) == next_avc)
                 continue;
 
-            if (length > 0) {
-                extract_length = length;
+            if (bytestream2_get_bytes_left(&bc) > 0) {
+                extract_length = bytestream2_get_bytes_left(&bc);
             } else if (pkt->nb_nals == 0) {
                 av_log(logctx, AV_LOG_ERROR, "No NAL unit found\n");
                 return AVERROR_INVALIDDATA;
@@ -286,14 +287,15 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length,
         }
         nal = &pkt->nals[pkt->nb_nals++];
 
-        consumed = ff_h2645_extract_rbsp(buf, extract_length, nal);
+        consumed = ff_h2645_extract_rbsp(bc.buffer, extract_length, nal);
         if (consumed < 0)
             return consumed;
 
+        bytestream2_skip(&bc, consumed);
+
         /* see commit 3566042a0 */
-        if (consumed < length - 3 &&
-            buf[consumed]     == 0x00 && buf[consumed + 1] == 0x00 &&
-            buf[consumed + 2] == 0x01 && buf[consumed + 3] == 0xE0)
+        if (bytestream2_get_bytes_left(&bc) >= 4 &&
+            bytestream2_peek_be32(&bc) == 0x000001E0)
             skip_trailing_zeros = 0;
 
         nal->size_bits = get_bit_length(nal, skip_trailing_zeros);
@@ -313,9 +315,6 @@ int ff_h2645_packet_split(H2645Packet *pkt, const uint8_t *buf, int length,
             }
             pkt->nb_nals--;
         }
-
-        buf    += consumed;
-        length -= consumed;
     }
 
     return 0;