Re: [libav-devel] [PATCH 3/4] h2645_parse: use the bytestream2 API for packet splitting

[Vittorio Giovara]

On Wed, Dec 28, 2016 at 1:15 PM, Anton Khirnov  wrote:
> 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-sta...@libav.org
> Bug-Id: 1002
> Found-By: Kamil Frankowicz
> ---
>  libavcodec/h2645_parse.c | 43 +--
>  1 file changed, 21 insertions(+), 22 deletions(-)
>

should be ok
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 3/4] h2645_parse: use the bytestream2 API for packet splitting

[Anton Khirnov]

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-sta...@libav.org
Bug-Id: 1002
Found-By: Kamil Frankowicz
---
 libavcodec/h2645_parse.c | 43 +--
 1 file 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(, buf, length);
 
 pkt->nb_nals = 0;
-while (length >= 4) {
+while (bytestream2_get_bytes_left() >= 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() == 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();
 
-if (extract_length > length) {
+if (extract_length > bytestream2_get_bytes_left()) {
 av_log(logctx, AV_LOG_ERROR,
"Invalid NAL unit size (%d > %d).\n",
-   extract_length, length);
+   extract_length, bytestream2_get_bytes_left());
 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() + 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(, buf_index);
 
 /*
  * break if an AVC1 length field is expected at the current buffer
  * position
  */
-if (buf == next_avc)
+if (bytestream2_tell() == next_avc)
 continue;
 
-if (length > 0) {
-extract_length = length;
+if (bytestream2_get_bytes_left() > 0) {
+extract_length = bytestream2_get_bytes_left();
 } 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 = >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(, 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() >= 4 &&
+bytestream2_peek_be32() == 0x01E0)
 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;
-- 
2.0.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel