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;