diff --git a/components/esp_driver_jpeg/jpeg_decode.c b/components/esp_driver_jpeg/jpeg_decode.c index 7a9ee9bd5a8..7bb7cae0a36 100644 --- a/components/esp_driver_jpeg/jpeg_decode.c +++ b/components/esp_driver_jpeg/jpeg_decode.c @@ -288,8 +288,14 @@ esp_err_t jpeg_decoder_process(jpeg_decoder_handle_t decoder_engine, const jpeg_ ESP_GOTO_ON_ERROR(jpeg_parse_header_info_to_hw(decoder_engine), err2, TAG, "write header info to hw failed"); ESP_GOTO_ON_ERROR(jpeg_dec_config_dma_descriptor(decoder_engine), err2, TAG, "config dma descriptor failed"); - *out_size = decoder_engine->header_info->process_h * decoder_engine->header_info->process_v * decoder_engine->bit_per_pixel / 8; - ESP_GOTO_ON_FALSE((*out_size <= outbuf_size), ESP_ERR_INVALID_ARG, err2, TAG, "Given buffer size % " PRId32 " is smaller than actual jpeg decode output size % " PRId32 "the height and width of output picture size will be adjusted to 16 bytes aligned automatically", outbuf_size, *out_size); + // Validate the decoded size against the output buffer unconditionally. Computed in + // 64-bit to avoid uint32_t wrap-around, and not gated on out_size, otherwise a NULL + // out_size would skip the check and let the DMA write past decode_outbuf. + uint64_t real_size = (uint64_t)decoder_engine->header_info->process_h * decoder_engine->header_info->process_v * decoder_engine->bit_per_pixel / 8; + ESP_GOTO_ON_FALSE((real_size <= outbuf_size), ESP_ERR_INVALID_ARG, err2, TAG, "Given buffer size %" PRIu32 " is smaller than actual jpeg decode output size %" PRIu64, outbuf_size, real_size); + if (out_size) { + *out_size = (uint32_t)real_size; + } dma2d_trans_config_t trans_desc = { .tx_channel_num = 1, diff --git a/components/esp_driver_jpeg/jpeg_parse_marker.c b/components/esp_driver_jpeg/jpeg_parse_marker.c index 50873ae0724..4dbc38b1a5d 100644 --- a/components/esp_driver_jpeg/jpeg_parse_marker.c +++ b/components/esp_driver_jpeg/jpeg_parse_marker.c @@ -152,16 +152,20 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) // Reject zero dimensions to avoid division-by-zero / degenerate buffers downstream ESP_RETURN_ON_FALSE(width != 0 && height != 0, ESP_ERR_INVALID_ARG, TAG, "Invalid picture size %ux%u", (unsigned)width, (unsigned)height); + // The 2D-DMA address fields are 14-bit, so each dimension must fit in JPEG_DMA2D_MAX_SIZE. + // This also bounds process_h * process_v and prevents the output-size integer overflow downstream. + ESP_RETURN_ON_FALSE(width <= JPEG_DMA2D_MAX_SIZE && height <= JPEG_DMA2D_MAX_SIZE, ESP_ERR_INVALID_ARG, TAG, "Picture length or height size %ux%u exceeds max %u", (unsigned)width, (unsigned)height, JPEG_DMA2D_MAX_SIZE); + if ((width * height % 8) != 0) { ESP_LOGE(TAG, "Picture sizes not divisible by 8 are not supported"); - return ESP_ERR_INVALID_STATE; + return ESP_ERR_NOT_SUPPORTED; } uint8_t nf = jpeg_get_bytes(header_info, 1); // Hardware supports 1..3 components (gray / YUV). nf must fit JPEG_COMPONENT_NUMBER_MAX-sized arrays. if (nf == 0 || nf >= JPEG_COMPONENT_NUMBER_MAX) { ESP_LOGE(TAG, "Only frame less than %d (and non-zero) is supported, got %u", JPEG_COMPONENT_NUMBER_MAX, nf); - return ESP_ERR_INVALID_STATE; + return ESP_ERR_NOT_SUPPORTED; } ESP_RETURN_ON_FALSE(lf >= 8 + 3 * nf, ESP_ERR_INVALID_ARG, TAG, "SOF length %"PRIu16" too small for %u components", lf, nf); @@ -193,6 +197,10 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) header_info->process_h = (uint32_t)(ceil(header_info->origin_h / header_info->mcux) + 1) * header_info->mcux; } + // process_h/process_v are written into the 14-bit 2D-DMA descriptor fields, so the + // MCU-rounded values (which may exceed the raw dimensions) must also fit the limit. + ESP_RETURN_ON_FALSE(header_info->process_h <= JPEG_DMA2D_MAX_SIZE && header_info->process_v <= JPEG_DMA2D_MAX_SIZE, ESP_ERR_INVALID_ARG, TAG, "MCU-aligned size %"PRIu32"x%"PRIu32" exceeds max %u", header_info->process_h, header_info->process_v, JPEG_DMA2D_MAX_SIZE); + return ESP_OK; } diff --git a/components/esp_driver_jpeg/test_apps/jpeg_test_apps/main/test_jpeg_decode.c b/components/esp_driver_jpeg/test_apps/jpeg_test_apps/main/test_jpeg_decode.c index e4b6d18c27b..dc23e07ed44 100644 --- a/components/esp_driver_jpeg/test_apps/jpeg_test_apps/main/test_jpeg_decode.c +++ b/components/esp_driver_jpeg/test_apps/jpeg_test_apps/main/test_jpeg_decode.c @@ -189,3 +189,56 @@ TEST_CASE("JPEG decode rejects malformed DQT index without crashing", "[jpeg]") free(tx_buf); TEST_ESP_OK(jpeg_del_decoder_engine(jpgd_handle)); } + +// Avoiding picture size is so large that exceeds the jpeg&dma limit. +static const uint8_t s_malformed_sof_mcu_round_jpg[] = { + 0xFF, 0xD8, // SOI + 0xFF, 0xC0, // SOF0 + 0x00, 0x0B, // Lf = 11 + 0x08, // P = 8 + 0x00, 0x10, // Y (height) = 16 + 0x3F, 0xFF, // X (width) = 16383 + 0x01, // Nf = 1 + 0x01, 0x22, 0x00, // component 1: Ci=1, H=2 V=2, Tq=0 +}; + +TEST_CASE("JPEG decode rejects MCU-rounded dimensions exceeding DMA limit", "[jpeg]") +{ + jpeg_decoder_handle_t jpgd_handle; + + jpeg_decode_engine_cfg_t decode_eng_cfg = { + .intr_priority = 0, + .timeout_ms = 80, + }; + + jpeg_decode_cfg_t decode_cfg = { + .output_format = JPEG_DECODE_OUT_FORMAT_RGB565, + }; + + jpeg_decode_memory_alloc_cfg_t rx_mem_cfg = { + .buffer_direction = JPEG_DEC_ALLOC_OUTPUT_BUFFER, + }; + + jpeg_decode_memory_alloc_cfg_t tx_mem_cfg = { + .buffer_direction = JPEG_DEC_ALLOC_INPUT_BUFFER, + }; + + size_t rx_buffer_size; + uint8_t *rx_buf = (uint8_t*)jpeg_alloc_decoder_mem(128, &rx_mem_cfg, &rx_buffer_size); + + size_t tx_buffer_size; + uint8_t *tx_buf = (uint8_t*)jpeg_alloc_decoder_mem(sizeof(s_malformed_sof_mcu_round_jpg), &tx_mem_cfg, &tx_buffer_size); + memcpy(tx_buf, s_malformed_sof_mcu_round_jpg, sizeof(s_malformed_sof_mcu_round_jpg)); + + TEST_ESP_OK(jpeg_new_decoder_engine(&decode_eng_cfg, &jpgd_handle)); + + uint32_t out_size = 0; + esp_err_t ret = jpeg_decoder_process(jpgd_handle, &decode_cfg, tx_buf, + sizeof(s_malformed_sof_mcu_round_jpg), rx_buf, + rx_buffer_size, &out_size); + TEST_ASSERT_NOT_EQUAL(ESP_OK, ret); + + free(rx_buf); + free(tx_buf); + TEST_ESP_OK(jpeg_del_decoder_engine(jpgd_handle)); +}