Add buffer bounds checks to av1_pack_bitstream()
Add the dest_size parameter to av1_write_uleb_obu_size(). The original
av1_write_uleb_obu_size() function is renamed
av1_write_uleb_obu_size_unsafe() and deprecated.
Add the data_size parameter to obu_memmove(). The original obu_memmove()
function is renamed obu_memmove_unsafe() and deprecated.
Add output buffer bounds checks to av1_write_metadata_obu(),
av1_write_metadata_array(), and av1_pack_bitstream().
Bug: 42302568, 361552711
Change-Id: Ib175e9344fbceb99879c3c79dbc37ad897264697
diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c
index 13e7177..ba13020 100644
--- a/av1/av1_cx_iface.c
+++ b/av1/av1_cx_iface.c
@@ -3446,7 +3446,8 @@
// OBUs are preceded/succeeded by an unsigned leb128 coded integer.
if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size,
- ctx->cx_data) != AOM_CODEC_OK) {
+ ctx->cx_data,
+ ctx->cx_data_sz) != AOM_CODEC_OK) {
aom_internal_error(&ppi->error, AOM_CODEC_ERROR, NULL);
}
@@ -3468,7 +3469,8 @@
memmove(cpi_data.cx_data + length_field_size, cpi_data.cx_data,
cpi_data.frame_size);
if (av1_write_uleb_obu_size(0, (uint32_t)cpi_data.frame_size,
- cpi_data.cx_data) != AOM_CODEC_OK) {
+ cpi_data.cx_data,
+ cpi_data.cx_data_sz) != AOM_CODEC_OK) {
aom_internal_error(&ppi->error, AOM_CODEC_ERROR, NULL);
}
cpi_data.frame_size += length_field_size;
@@ -3496,8 +3498,8 @@
size_t tu_size = ctx->pending_cx_data_sz;
const size_t length_field_size = aom_uleb_size_in_bytes(tu_size);
memmove(ctx->cx_data + length_field_size, ctx->cx_data, tu_size);
- if (av1_write_uleb_obu_size(0, (uint32_t)tu_size, ctx->cx_data) !=
- AOM_CODEC_OK) {
+ if (av1_write_uleb_obu_size(0, (uint32_t)tu_size, ctx->cx_data,
+ ctx->cx_data_sz) != AOM_CODEC_OK) {
aom_internal_error(&ppi->error, AOM_CODEC_ERROR, NULL);
}
ctx->pending_cx_data_sz += length_field_size;
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c
index b3868ba..a6e213b 100644
--- a/av1/encoder/bitstream.c
+++ b/av1/encoder/bitstream.c
@@ -14,6 +14,7 @@
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
+#include <string.h>
#include "aom/aom_encoder.h"
#include "aom_dsp/aom_dsp_common.h"
@@ -3403,7 +3404,23 @@
}
int av1_write_uleb_obu_size(size_t obu_header_size, size_t obu_payload_size,
- uint8_t *dest) {
+ uint8_t *dest, size_t dest_size) {
+ const size_t offset = obu_header_size;
+ size_t coded_obu_size = 0;
+
+ if (offset > dest_size) {
+ return AOM_CODEC_ERROR;
+ }
+ if (aom_uleb_encode(obu_payload_size, dest_size - offset, dest + offset,
+ &coded_obu_size) != 0) {
+ return AOM_CODEC_ERROR;
+ }
+
+ return AOM_CODEC_OK;
+}
+
+int av1_write_uleb_obu_size_unsafe(size_t obu_header_size,
+ size_t obu_payload_size, uint8_t *dest) {
const size_t offset = obu_header_size;
size_t coded_obu_size = 0;
@@ -3415,8 +3432,23 @@
return AOM_CODEC_OK;
}
+// Returns 0 on failure.
static size_t obu_memmove(size_t obu_header_size, size_t obu_payload_size,
- uint8_t *data) {
+ uint8_t *data, size_t data_size) {
+ const size_t length_field_size = aom_uleb_size_in_bytes(obu_payload_size);
+ const size_t move_dst_offset = length_field_size + obu_header_size;
+ const size_t move_src_offset = obu_header_size;
+ const size_t move_size = obu_payload_size;
+ if (move_dst_offset + move_size > data_size) {
+ return 0;
+ }
+ memmove(data + move_dst_offset, data + move_src_offset, move_size);
+ return length_field_size;
+}
+
+// Deprecated. Use obu_memmove() instead.
+static size_t obu_memmove_unsafe(size_t obu_header_size,
+ size_t obu_payload_size, uint8_t *data) {
const size_t length_field_size = aom_uleb_size_in_bytes(obu_payload_size);
const size_t move_dst_offset = length_field_size + obu_header_size;
const size_t move_src_offset = obu_header_size;
@@ -3441,7 +3473,9 @@
}
uint32_t av1_write_sequence_header_obu(const SequenceHeader *seq_params,
- uint8_t *const dst) {
+ uint8_t *const dst, size_t dst_size) {
+ // TODO: bug 42302568 - Use dst_size.
+ (void)dst_size;
struct aom_write_bit_buffer wb = { dst, 0 };
uint32_t size = 0;
@@ -3604,9 +3638,9 @@
*total_size += lst_obu->tg_hdr_size;
const uint32_t obu_payload_size = *total_size - lst_obu->tg_hdr_size;
const size_t length_field_size =
- obu_memmove(lst_obu->tg_hdr_size, obu_payload_size, dst);
- if (av1_write_uleb_obu_size(lst_obu->tg_hdr_size, obu_payload_size, dst) !=
- AOM_CODEC_OK)
+ obu_memmove_unsafe(lst_obu->tg_hdr_size, obu_payload_size, dst);
+ if (av1_write_uleb_obu_size_unsafe(lst_obu->tg_hdr_size, obu_payload_size,
+ dst) != AOM_CODEC_OK)
assert(0);
*total_size += (uint32_t)length_field_size;
@@ -3830,9 +3864,9 @@
// write current tile group size
const size_t obu_payload_size = *curr_tg_data_size - obu_header_size;
const size_t length_field_size =
- obu_memmove(obu_header_size, obu_payload_size, curr_tg_start);
- if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size,
- curr_tg_start) != AOM_CODEC_OK) {
+ obu_memmove_unsafe(obu_header_size, obu_payload_size, curr_tg_start);
+ if (av1_write_uleb_obu_size_unsafe(obu_header_size, obu_payload_size,
+ curr_tg_start) != AOM_CODEC_OK) {
aom_internal_error(cpi->common.error, AOM_CODEC_ERROR,
"av1_write_last_tile_info: output buffer full");
}
@@ -4104,10 +4138,13 @@
}
static uint32_t write_tiles_in_tg_obus(AV1_COMP *const cpi, uint8_t *const dst,
+ size_t dst_size,
struct aom_write_bit_buffer *saved_wb,
uint8_t obu_extension_header,
const FrameHeaderInfo *fh_info,
int *const largest_tile_id) {
+ // TODO: bug 42302568 - Use dst_size.
+ (void)dst_size;
AV1_COMMON *const cm = &cpi->common;
const CommonTileParams *const tiles = &cm->tiles;
*largest_tile_id = 0;
@@ -4132,12 +4169,16 @@
fh_info, largest_tile_id);
}
+// Returns the number of bytes written on success. Returns 0 on failure.
static size_t av1_write_metadata_obu(const aom_metadata_t *metadata,
- uint8_t *const dst) {
+ uint8_t *const dst, size_t dst_size) {
size_t coded_metadata_size = 0;
const uint64_t metadata_type = (uint64_t)metadata->type;
- if (aom_uleb_encode(metadata_type, sizeof(metadata_type), dst,
- &coded_metadata_size) != 0) {
+ if (aom_uleb_encode(metadata_type, dst_size, dst, &coded_metadata_size) !=
+ 0) {
+ return 0;
+ }
+ if (coded_metadata_size + metadata->sz + 1 > dst_size) {
return 0;
}
memcpy(dst + coded_metadata_size, metadata->payload, metadata->sz);
@@ -4146,7 +4187,8 @@
return coded_metadata_size + metadata->sz + 1;
}
-static size_t av1_write_metadata_array(AV1_COMP *const cpi, uint8_t *dst) {
+static size_t av1_write_metadata_array(AV1_COMP *const cpi, uint8_t *dst,
+ size_t dst_size) {
if (!cpi->source) return 0;
AV1_COMMON *const cm = &cpi->common;
aom_metadata_array_t *arr = cpi->source->metadata;
@@ -4163,20 +4205,38 @@
(cm->current_frame.frame_type != KEY_FRAME &&
current_metadata->insert_flag == AOM_MIF_NON_KEY_FRAME) ||
current_metadata->insert_flag == AOM_MIF_ANY_FRAME) {
+ // OBU header is either one or two bytes.
+ if (dst_size < 2) {
+ aom_internal_error(cm->error, AOM_CODEC_ERROR,
+ "av1_write_metadata_array: output buffer full");
+ }
obu_header_size = av1_write_obu_header(
&cpi->ppi->level_params, &cpi->frame_header_count, OBU_METADATA,
cm->seq_params->has_nonzero_operating_point_idc, 0, dst);
+ assert(obu_header_size <= 2);
obu_payload_size =
- av1_write_metadata_obu(current_metadata, dst + obu_header_size);
- length_field_size = obu_memmove(obu_header_size, obu_payload_size, dst);
- if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, dst) ==
- AOM_CODEC_OK) {
- const size_t obu_size = obu_header_size + obu_payload_size;
- dst += obu_size + length_field_size;
- total_bytes_written += obu_size + length_field_size;
+ av1_write_metadata_obu(current_metadata, dst + obu_header_size,
+ dst_size - obu_header_size);
+ if (obu_payload_size == 0) {
+ aom_internal_error(cm->error, AOM_CODEC_ERROR,
+ "av1_write_metadata_array: output buffer full");
+ }
+ length_field_size =
+ obu_memmove(obu_header_size, obu_payload_size, dst, dst_size);
+ if (length_field_size == 0) {
+ aom_internal_error(cm->error, AOM_CODEC_ERROR,
+ "av1_write_metadata_array: output buffer full");
+ }
+ if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, dst,
+ dst_size) == AOM_CODEC_OK) {
+ const size_t obu_size =
+ obu_header_size + obu_payload_size + length_field_size;
+ dst += obu_size;
+ dst_size -= obu_size;
+ total_bytes_written += obu_size;
} else {
aom_internal_error(cpi->common.error, AOM_CODEC_ERROR,
- "Error writing metadata OBU size");
+ "av1_write_metadata_array: output buffer full");
}
}
}
@@ -4187,7 +4247,7 @@
int av1_pack_bitstream(AV1_COMP *const cpi, uint8_t *dst, size_t dst_size,
size_t *size, int *const largest_tile_id) {
uint8_t *data = dst;
- (void)dst_size;
+ size_t data_size = dst_size;
AV1_COMMON *const cm = &cpi->common;
AV1LevelParams *const level_params = &cpi->ppi->level_params;
uint32_t obu_header_size = 0;
@@ -4213,23 +4273,38 @@
// preceded by 4-byte size
if (cm->current_frame.frame_type == INTRA_ONLY_FRAME ||
cm->current_frame.frame_type == KEY_FRAME) {
+ // OBU header is either one or two bytes.
+ if (data_size < 2) {
+ return AOM_CODEC_ERROR;
+ }
obu_header_size = av1_write_obu_header(
level_params, &cpi->frame_header_count, OBU_SEQUENCE_HEADER,
cm->seq_params->has_nonzero_operating_point_idc, 0, data);
- obu_payload_size =
- av1_write_sequence_header_obu(cm->seq_params, data + obu_header_size);
+ assert(obu_header_size <= 2);
+ obu_payload_size = av1_write_sequence_header_obu(
+ cm->seq_params, data + obu_header_size, data_size - obu_header_size);
const size_t length_field_size =
- obu_memmove(obu_header_size, obu_payload_size, data);
- if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, data) !=
- AOM_CODEC_OK) {
+ obu_memmove(obu_header_size, obu_payload_size, data, data_size);
+ if (length_field_size == 0) {
+ return AOM_CODEC_ERROR;
+ }
+ if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, data,
+ data_size) != AOM_CODEC_OK) {
return AOM_CODEC_ERROR;
}
- data += obu_header_size + obu_payload_size + length_field_size;
+ const size_t bytes_written =
+ obu_header_size + obu_payload_size + length_field_size;
+ data += bytes_written;
+ data_size -= bytes_written;
}
// write metadata obus before the frame obu that has the show_frame flag set
- if (cm->show_frame) data += av1_write_metadata_array(cpi, data);
+ if (cm->show_frame) {
+ const size_t bytes_written = av1_write_metadata_array(cpi, data, data_size);
+ data += bytes_written;
+ data_size -= bytes_written;
+ }
const int write_frame_header =
(cpi->num_tg > 1 || encode_show_existing_frame(cm));
@@ -4238,16 +4313,26 @@
if (write_frame_header) {
// Write Frame Header OBU.
fh_info.frame_header = data;
+ // OBU header is either one or two bytes.
+ if (data_size < 2) {
+ return AOM_CODEC_ERROR;
+ }
obu_header_size = av1_write_obu_header(
level_params, &cpi->frame_header_count, OBU_FRAME_HEADER,
cm->seq_params->has_nonzero_operating_point_idc, obu_extension_header,
data);
+ // TODO: bug 42302568 - Pass data_size - obu_header_size to
+ // write_frame_header_obu().
obu_payload_size = write_frame_header_obu(cpi, &cpi->td.mb.e_mbd, &saved_wb,
data + obu_header_size, 1);
- length_field = obu_memmove(obu_header_size, obu_payload_size, data);
- if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, data) !=
- AOM_CODEC_OK) {
+ length_field =
+ obu_memmove(obu_header_size, obu_payload_size, data, data_size);
+ if (length_field == 0) {
+ return AOM_CODEC_ERROR;
+ }
+ if (av1_write_uleb_obu_size(obu_header_size, obu_payload_size, data,
+ data_size) != AOM_CODEC_OK) {
return AOM_CODEC_ERROR;
}
@@ -4258,12 +4343,10 @@
return AOM_CODEC_ERROR;
}
data += fh_info.total_length;
+ data_size -= fh_info.total_length;
}
- uint32_t data_size;
- if (encode_show_existing_frame(cm)) {
- data_size = 0;
- } else {
+ if (!encode_show_existing_frame(cm)) {
// Since length_field is determined adaptively after frame header
// encoding, saved_wb must be adjusted accordingly.
if (saved_wb.bit_buffer != NULL) {
@@ -4272,10 +4355,13 @@
// Each tile group obu will be preceded by 4-byte size of the tile group
// obu
- data_size = write_tiles_in_tg_obus(
- cpi, data, &saved_wb, obu_extension_header, &fh_info, largest_tile_id);
+ const size_t bytes_written =
+ write_tiles_in_tg_obus(cpi, data, data_size, &saved_wb,
+ obu_extension_header, &fh_info, largest_tile_id);
+ data += bytes_written;
+ data_size -= bytes_written;
}
- data += data_size;
*size = data - dst;
+ (void)data_size;
return AOM_CODEC_OK;
}
diff --git a/av1/encoder/bitstream.h b/av1/encoder/bitstream.h
index ec213ef..0a45b5f 100644
--- a/av1/encoder/bitstream.h
+++ b/av1/encoder/bitstream.h
@@ -17,6 +17,8 @@
#endif
#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
#include "av1/common/av1_common_int.h"
#include "av1/common/blockd.h"
@@ -88,7 +90,7 @@
// payload written to 'dst'. This function does not write the OBU header, the
// optional extension, or the OBU size to 'dst'.
uint32_t av1_write_sequence_header_obu(const SequenceHeader *seq_params,
- uint8_t *const dst);
+ uint8_t *const dst, size_t dst_size);
// Writes the OBU header byte, and the OBU header extension byte when
// has_nonzero_operating_point_idc is true and the OBU is part of a frame.
@@ -99,7 +101,11 @@
int obu_extension, uint8_t *const dst);
int av1_write_uleb_obu_size(size_t obu_header_size, size_t obu_payload_size,
- uint8_t *dest);
+ uint8_t *dest, size_t dest_size);
+
+// Deprecated. Use av1_write_uleb_obu_size() instead.
+int av1_write_uleb_obu_size_unsafe(size_t obu_header_size,
+ size_t obu_payload_size, uint8_t *dest);
// Pack tile data in the bitstream with tile_group, frame
// and OBU header.
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c
index b356b3f..bbca48a 100644
--- a/av1/encoder/encoder.c
+++ b/av1/encoder/encoder.c
@@ -5400,8 +5400,8 @@
if (!ppi) return NULL;
uint8_t header_buf[512] = { 0 };
- const uint32_t sequence_header_size =
- av1_write_sequence_header_obu(&ppi->seq_params, &header_buf[0]);
+ const uint32_t sequence_header_size = av1_write_sequence_header_obu(
+ &ppi->seq_params, &header_buf[0], sizeof(header_buf));
assert(sequence_header_size <= sizeof(header_buf));
if (sequence_header_size == 0) return NULL;
