From 5f95ab68f90211b63accb91deb7964e3b722df01 Mon Sep 17 00:00:00 2001 From: "C.S.M" Date: Tue, 26 May 2026 12:11:22 +0800 Subject: [PATCH] fix(jpeg_decoder): Add some strict check to avoid bad picture attack --- components/esp_driver_jpeg/jpeg_decode.c | 111 +++++++++++++++--- .../esp_driver_jpeg/jpeg_parse_marker.c | 105 +++++++++++++---- components/esp_driver_jpeg/jpeg_private.h | 1 + .../jpeg_test_apps/main/test_jpeg_decode.c | 56 +++++++++ 4 files changed, 231 insertions(+), 42 deletions(-) diff --git a/components/esp_driver_jpeg/jpeg_decode.c b/components/esp_driver_jpeg/jpeg_decode.c index 50efd00c416..296870b632e 100644 --- a/components/esp_driver_jpeg/jpeg_decode.c +++ b/components/esp_driver_jpeg/jpeg_decode.c @@ -140,6 +140,7 @@ esp_err_t jpeg_decoder_get_info(const uint8_t *in_buf, uint32_t inbuf_len, jpeg_ { ESP_RETURN_ON_FALSE(in_buf, ESP_ERR_INVALID_ARG, TAG, "jpeg decode input buffer is NULL"); ESP_RETURN_ON_FALSE(inbuf_len != 0, ESP_ERR_INVALID_ARG, TAG, "jpeg decode input buffer length is 0"); + ESP_RETURN_ON_FALSE(picture_info, ESP_ERR_INVALID_ARG, TAG, "jpeg decode picture_info is NULL"); jpeg_dec_header_info_t* header_info = (jpeg_dec_header_info_t*)heap_caps_calloc(1, sizeof(jpeg_dec_header_info_t), JPEG_MEM_ALLOC_CAPS); ESP_RETURN_ON_FALSE(header_info, ESP_ERR_NO_MEM, TAG, "no memory for picture info"); @@ -148,33 +149,85 @@ esp_err_t jpeg_decoder_get_info(const uint8_t *in_buf, uint32_t inbuf_len, jpeg_ header_info->header_size = 0; uint16_t height = 0; uint16_t width = 0; - uint8_t thischar = 0; - uint8_t lastchar = 0; uint8_t hivi = 0; uint8_t nf = 0; + bool sof_found = false; + esp_err_t ret = ESP_OK; - while (header_info->buffer_left) { - lastchar = thischar; - thischar = jpeg_get_bytes(header_info, 1); - uint16_t marker = (lastchar << 8 | thischar); - switch (marker) { - case JPEG_M_SOF0: + while (header_info->buffer_left >= 2) { + uint8_t b0 = jpeg_get_bytes(header_info, 1); + if (b0 != 0xFF) { + continue; + } + uint8_t b1 = jpeg_get_bytes(header_info, 1); + // A marker may be preceded by any number of 0xFF fill bytes + // (T.81 B.1.1.2). Consume the run of fill bytes so b1 lands on + // the marker code; e.g. "FF FF C0" must parse as marker 0xFFC0. + while (b1 == 0xFF) { + if (header_info->buffer_left < 1) { + break; + } + b1 = jpeg_get_bytes(header_info, 1); + } + if (b1 == 0x00 || b1 == 0xFF) { + continue; + } + uint16_t marker = (uint16_t)0xFF00u | b1; + + if (marker == JPEG_M_SOF0) { + // Need Lf(2)+P(1)+Y(2)+X(2)+Nf(1)+at least 3 bytes of the first component + if (header_info->buffer_left < 11) { + ESP_LOGE(TAG, "Truncated SOF0 segment"); + ret = ESP_ERR_INVALID_ARG; + goto out; + } jpeg_get_bytes(header_info, 2); jpeg_get_bytes(header_info, 1); height = jpeg_get_bytes(header_info, 2); width = jpeg_get_bytes(header_info, 2); - nf = jpeg_get_bytes(header_info, 1); - jpeg_get_bytes(header_info, 1); hivi = jpeg_get_bytes(header_info, 1); + sof_found = true; break; } - // This function only used for get width and height. So only read SOF marker is enough. - // Can be extended if picture information is extended. - if (marker == JPEG_M_SOF0) { + + // Standalone markers (no length field): SOI, EOI, RST0..RST7. + if (marker == JPEG_M_SOI || (b1 >= 0xD0 && b1 <= 0xD7)) { + continue; + } + if (marker == JPEG_M_EOI) { + // Reached end of image without SOF0; bail out. break; } + // SOS appearing before SOF0 is malformed for this query API. + if (marker == JPEG_M_SOS) { + ESP_LOGE(TAG, "SOS encountered before SOF0"); + ret = ESP_ERR_NOT_FOUND; + goto out; + } + + // All remaining markers (SOFx variants, DHT, DQT, DRI, APPn, COM, ...) + // carry a 2-byte big-endian length immediately after the marker byte. + if (header_info->buffer_left < 2) { + break; + } + uint16_t seg_len = jpeg_get_bytes(header_info, 2); + if (seg_len < 2 || header_info->buffer_left < (uint32_t)(seg_len - 2)) { + ESP_LOGE(TAG, "Truncated/invalid segment for marker 0x%04x, len=%u", marker, seg_len); + ret = ESP_ERR_INVALID_ARG; + goto out; + } + uint16_t to_skip = seg_len - 2; + header_info->buffer_offset += to_skip; + header_info->header_size += to_skip; + header_info->buffer_left -= to_skip; + } + + if (!sof_found) { + ESP_LOGE(TAG, "SOF0 marker not found"); + ret = ESP_ERR_NOT_FOUND; + goto out; } picture_info->height = height; @@ -193,15 +246,20 @@ esp_err_t jpeg_decoder_get_info(const uint8_t *in_buf, uint32_t inbuf_len, jpeg_ break; default: ESP_LOGE(TAG, "Sampling factor cannot be recognized"); - return ESP_ERR_INVALID_STATE; + ret = ESP_ERR_INVALID_STATE; + goto out; } - } - if (nf == 1) { + } else if (nf == 1) { picture_info->sample_method = JPEG_DOWN_SAMPLING_GRAY; + } else { + ESP_LOGE(TAG, "Unsupported number of frame components: %u", nf); + ret = ESP_ERR_INVALID_STATE; + goto out; } +out: free(header_info); - return ESP_OK; + return ret; } static bool _check_buffer_alignment(void *buffer, uint32_t buffer_size, uint32_t alignment) @@ -740,7 +798,10 @@ static esp_err_t jpeg_parse_marker(jpeg_decoder_handle_t decoder_engine, const u jpeg_ll_set_picture_height(hal->dev, 0); jpeg_ll_set_picture_width(hal->dev, 0); - while (header_info->buffer_left) { + // Loop guard requires >=2 bytes so the 2-byte marker read can never underflow. + bool sof_found = false; + bool sos_found = false; + while (header_info->buffer_left >= 2) { uint8_t lastchar = jpeg_get_bytes(header_info, 1); uint8_t thischar = jpeg_get_bytes(header_info, 1); uint16_t marker = (lastchar << 8 | thischar); @@ -773,6 +834,7 @@ static esp_err_t jpeg_parse_marker(jpeg_decoder_handle_t decoder_engine, const u break; case JPEG_M_SOF0: ESP_RETURN_ON_ERROR(jpeg_parse_sof_marker(header_info), TAG, "deal sof marker failed"); + sof_found = true; break; case JPEG_M_SOF1: case JPEG_M_SOF2: @@ -796,16 +858,27 @@ static esp_err_t jpeg_parse_marker(jpeg_decoder_handle_t decoder_engine, const u break; case JPEG_M_SOS: ESP_RETURN_ON_ERROR(jpeg_parse_sos_marker(header_info), TAG, "deal sos marker failed"); + sos_found = true; break; case JPEG_M_INV: ESP_RETURN_ON_ERROR(jpeg_parse_inv_marker(header_info), TAG, "deal invalid marker failed"); break; + default: + // Reject unknown/unsupported markers instead of silently continuing. + ESP_LOGE(TAG, "Unsupported or unknown marker 0x%04x", marker); + return ESP_ERR_INVALID_ARG; } - if (marker == JPEG_M_SOS) { + if (sos_found) { break; } } + // SOF0 must precede SOS; without it nf/dimensions/mcu fields would remain zero + // from memset and the hardware would be configured with garbage. + ESP_RETURN_ON_FALSE(sof_found, ESP_ERR_INVALID_ARG, TAG, "SOF0 marker not found in JPEG stream"); + // The dispatcher must terminate via SOS; otherwise the stream is truncated/invalid. + ESP_RETURN_ON_FALSE(sos_found, ESP_ERR_INVALID_ARG, TAG, "SOS marker not found before end of stream"); + // Update information after parse marker finishes decoder_engine->header_info->buffer_left = decoder_engine->total_size - decoder_engine->header_info->header_size; diff --git a/components/esp_driver_jpeg/jpeg_parse_marker.c b/components/esp_driver_jpeg/jpeg_parse_marker.c index 5dd410fe21b..32c3f9cd395 100644 --- a/components/esp_driver_jpeg/jpeg_parse_marker.c +++ b/components/esp_driver_jpeg/jpeg_parse_marker.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -44,8 +44,10 @@ uint32_t jpeg_get_bytes(jpeg_dec_header_info_t *header_info, uint8_t num_bytes) esp_err_t jpeg_parse_appn_marker(jpeg_dec_header_info_t *header_info) { + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "APPn marker truncated: missing length"); uint16_t skip_num = jpeg_get_bytes(header_info, 2); - ESP_RETURN_ON_FALSE(skip_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid APPn marker length: %"PRIu32, skip_num); + ESP_RETURN_ON_FALSE(skip_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid APPn marker length: %"PRIu16, skip_num); uint16_t bytes_to_skip = skip_num - 2; ESP_RETURN_ON_FALSE(header_info->buffer_left >= bytes_to_skip, ESP_ERR_INVALID_ARG, TAG, "APPn marker data underflow for buffer_left: %"PRIu32, header_info->buffer_left); header_info->buffer_offset += bytes_to_skip; @@ -57,10 +59,12 @@ esp_err_t jpeg_parse_appn_marker(jpeg_dec_header_info_t *header_info) esp_err_t jpeg_parse_com_marker(jpeg_dec_header_info_t *header_info) { + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "COM marker truncated: missing length"); uint16_t skip_num = jpeg_get_bytes(header_info, 2); - ESP_RETURN_ON_FALSE(skip_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid COM marker length: %"PRIu32, skip_num); + ESP_RETURN_ON_FALSE(skip_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid COM marker length: %"PRIu16, skip_num); uint32_t bytes_to_skip = skip_num - 2; - ESP_RETURN_ON_FALSE(header_info->buffer_left >= bytes_to_skip, ESP_ERR_INVALID_ARG, TAG, "COM marker data underflow for header_size: %"PRIu32, header_info->buffer_left); + ESP_RETURN_ON_FALSE(header_info->buffer_left >= bytes_to_skip, ESP_ERR_INVALID_ARG, TAG, "COM marker data underflow for buffer_left: %"PRIu32, header_info->buffer_left); header_info->buffer_offset += bytes_to_skip; header_info->header_size += bytes_to_skip; header_info->buffer_left -= bytes_to_skip; @@ -72,30 +76,49 @@ esp_err_t jpeg_parse_dqt_marker(jpeg_dec_header_info_t *header_info) uint32_t n = 0, i = 0, prec = 0; uint32_t temp = 0; + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "DQT marker truncated: missing length"); uint16_t length_num = jpeg_get_bytes(header_info, 2); - ESP_RETURN_ON_FALSE(length_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid DQT marker length: %"PRIu32, length_num); + ESP_RETURN_ON_FALSE(length_num >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid DQT marker length: %"PRIu16, length_num); length_num -= 2; + ESP_RETURN_ON_FALSE(header_info->buffer_left >= length_num, ESP_ERR_INVALID_ARG, TAG, "DQT marker truncated: buffer_left=%"PRIu32" needed=%"PRIu16, header_info->buffer_left, length_num); while (length_num) { + // Need at least 1 byte for the table identifier (Pq | Tq) + ESP_RETURN_ON_FALSE(length_num >= 1, ESP_ERR_INVALID_ARG, TAG, "DQT marker length error before reading id"); n = jpeg_get_bytes(header_info, 1); + length_num -= 1; prec = n >> 4; n &= 0x0F; - ESP_RETURN_ON_FALSE(length_num >= 1, ESP_ERR_INVALID_ARG, TAG, "DQT marker length error: %"PRIu32, length_num); - length_num -= 1; - // read quantization entries, in zig-zag order + ESP_RETURN_ON_FALSE(n < JPEG_COMPONENT_NUMBER_MAX, ESP_ERR_INVALID_ARG, TAG, "DQT marker: invalid quantization table id %"PRIu32, n); + // Pq must be 0 (8-bit) or 1 (16-bit); other values are reserved and unsupported + ESP_RETURN_ON_FALSE(prec <= 1, ESP_ERR_INVALID_ARG, TAG, "DQT marker: invalid precision %"PRIu32, prec); + + // Each table needs 64 * (prec+1) bytes; verify upfront before consuming + const uint32_t entry_bytes = 64 * (prec + 1); + ESP_RETURN_ON_FALSE(length_num >= entry_bytes, ESP_ERR_INVALID_ARG, TAG, "DQT marker truncated: need %"PRIu32" remaining %"PRIu16, entry_bytes, length_num); + + // Read quantization entries, in zig-zag order for (i = 0; i < 64; i++) { temp = jpeg_get_bytes(header_info, 1); - ESP_RETURN_ON_FALSE(length_num >= 1, ESP_ERR_INVALID_ARG, TAG, "DQT marker length error: %"PRIu32, length_num); length_num -= 1; if (prec) { temp = (temp << 8) + jpeg_get_bytes(header_info, 1); - ESP_RETURN_ON_FALSE(length_num >= 1, ESP_ERR_INVALID_ARG, TAG, "DQT marker length error: %"PRIu32, length_num); length_num -= 1; } header_info->qt_tbl[n][zigzag_arr[i]] = temp; } - header_info->qt_tbl_num++; + + // Only count distinct Tq IDs; a repeated ID (in this segment or an + // earlier DQT segment) rewrites qt_tbl[n] but must not advance + // qt_tbl_num past the number of populated slots. + const uint8_t tq_bit = (uint8_t)(1 << n); + if ((header_info->qt_tbl_seen_mask & tq_bit) == 0) { + ESP_RETURN_ON_FALSE(header_info->qt_tbl_num < JPEG_COMPONENT_NUMBER_MAX, ESP_ERR_INVALID_ARG, TAG, "DQT marker: too many quantization tables"); + header_info->qt_tbl_seen_mask |= tq_bit; + header_info->qt_tbl_num++; + } } return ESP_OK; @@ -104,7 +127,15 @@ esp_err_t jpeg_parse_dqt_marker(jpeg_dec_header_info_t *header_info) esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) { - jpeg_get_bytes(header_info, 2); + // SOF segment layout (excluding the 0xFFCx marker bytes): + // Lf(2) + P(1) + Y(2) + X(2) + Nf(1) + Nf * [Ci(1) + Hi|Vi(1) + Tqi(1)] + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "SOF marker truncated: missing length"); + uint16_t lf = jpeg_get_bytes(header_info, 2); + ESP_RETURN_ON_FALSE(lf >= 8, ESP_ERR_INVALID_ARG, TAG, "Invalid SOF marker length: %"PRIu16, lf); + uint16_t remaining = lf - 2; + ESP_RETURN_ON_FALSE(header_info->buffer_left >= remaining, ESP_ERR_INVALID_ARG, TAG, "SOF marker truncated: buffer_left=%"PRIu32" need=%"PRIu16, header_info->buffer_left, remaining); + if (jpeg_get_bytes(header_info, 1) != 8) { ESP_LOGE(TAG, "Sample precision is not 8"); return ESP_ERR_INVALID_STATE; @@ -118,17 +149,23 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) header_info->origin_h = width; header_info->process_h = width; + // 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); + if ((width * height % 8) != 0) { ESP_LOGE(TAG, "Picture sizes not divisible by 8 are not supported"); return ESP_ERR_INVALID_STATE; } uint8_t nf = jpeg_get_bytes(header_info, 1); - if (nf >= 4 || nf == 0) { - ESP_LOGE(TAG, "Only frame less or equal than 4 is supported."); + // 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; } + ESP_RETURN_ON_FALSE(lf >= 8 + 3 * nf, ESP_ERR_INVALID_ARG, TAG, "SOF length %"PRIu16" too small for %u components", lf, nf); + header_info->nf = nf; for (int i = 0; i < nf; i++) { @@ -137,8 +174,13 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) header_info->vi[i] = header_info->hivi[i] & 0x0f; header_info->hi[i] = (header_info->hivi[i] & 0xf0) >> 4; header_info->qtid[i] = jpeg_get_bytes(header_info, 1); + // qtid selects the quantization table that the hardware will use; bound it. + ESP_RETURN_ON_FALSE(header_info->qtid[i] < JPEG_COMPONENT_NUMBER_MAX, ESP_ERR_INVALID_ARG, TAG, "SOF: invalid Tq[%d]=%u", i, header_info->qtid[i]); } + // Guard against zero sampling factors before we use them as a divisor below. + ESP_RETURN_ON_FALSE(header_info->hi[0] != 0 && header_info->vi[0] != 0, ESP_ERR_INVALID_ARG, TAG, "SOF: zero sampling factor H=%u V=%u", header_info->hi[0], header_info->vi[0]); + // Set MCU block pixel according to factor. (For 3 components, we only use Y factor) header_info->mcux = header_info->hi[0] * 8; header_info->mcuy = header_info->vi[0] * 8; @@ -157,28 +199,42 @@ esp_err_t jpeg_parse_sof_marker(jpeg_dec_header_info_t *header_info) esp_err_t jpeg_parse_dht_marker(jpeg_dec_header_info_t *header_info) { // Recording num_left in DHT sector, not including length bytes (2 bytes). + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "DHT marker truncated: missing length"); uint16_t raw_length = jpeg_get_bytes(header_info, 2); // Check for integer underflow before subtraction - ESP_RETURN_ON_FALSE(raw_length >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid DHT marker length: %"PRIu32, raw_length); + ESP_RETURN_ON_FALSE(raw_length >= 2, ESP_ERR_INVALID_ARG, TAG, "Invalid DHT marker length: %"PRIu16, raw_length); uint16_t num_left = raw_length - 2; + ESP_RETURN_ON_FALSE(header_info->buffer_left >= num_left, ESP_ERR_INVALID_ARG, TAG, "DHT marker truncated: buffer_left=%"PRIu32" needed=%"PRIu16, header_info->buffer_left, num_left); + while (num_left) { uint32_t np = 0; + uint8_t tc = 0; + uint8_t th = 0; + + // Need 1 byte (Tc | Th) + 16 bytes (Li) at minimum before reading values + ESP_RETURN_ON_FALSE(num_left >= (1 + JPEG_HUFFMAN_BITS_LEN_TABLE_LEN), ESP_ERR_INVALID_ARG, TAG, "DHT marker truncated: num_left=%"PRIu16, num_left); // Get information of huffman table header_info->huffinfo.info = jpeg_get_bytes(header_info, 1); + tc = header_info->huffinfo.type; + th = header_info->huffinfo.id; + + ESP_RETURN_ON_FALSE(tc < DHT_TC_NUM, ESP_ERR_INVALID_ARG, TAG, "DHT marker: invalid table class Tc=%u", tc); + ESP_RETURN_ON_FALSE(th < DHT_TH_NUM, ESP_ERR_INVALID_ARG, TAG, "DHT marker: invalid table id Th=%u", th); for (int i = 0; i < JPEG_HUFFMAN_BITS_LEN_TABLE_LEN; i++) { - header_info->huffbits[header_info->huffinfo.type][header_info->huffinfo.id][i] = jpeg_get_bytes(header_info, 1); + header_info->huffbits[tc][th][i] = jpeg_get_bytes(header_info, 1); // Record number of patterns. - np += header_info->huffbits[header_info->huffinfo.type][header_info->huffinfo.id][i]; + np += header_info->huffbits[tc][th][i]; + } + ESP_RETURN_ON_FALSE(np <= JPEG_HUFFMAN_AC_VALUE_TABLE_LEN, ESP_ERR_INVALID_ARG, TAG, "DHT marker: huffcode count %"PRIu32" exceeds table size", np); + ESP_RETURN_ON_FALSE(num_left >= (1u + JPEG_HUFFMAN_BITS_LEN_TABLE_LEN + np), ESP_ERR_INVALID_ARG, TAG, "DHT marker truncated for huffcode: num_left=%"PRIu16" need=%"PRIu32, num_left, (uint32_t)(1u + JPEG_HUFFMAN_BITS_LEN_TABLE_LEN + np)); + + for (uint32_t i = 0; i < np; i++) { + header_info->huffcode[tc][th][i] = jpeg_get_bytes(header_info, 1); } - for (int i = 0; i < np; i++) { - header_info->huffcode[header_info->huffinfo.type][header_info->huffinfo.id][i] = jpeg_get_bytes(header_info, 1); - } - - // Check for integer underflow before subtraction - ESP_RETURN_ON_FALSE(num_left >= (JPEG_HUFFMAN_BITS_LEN_TABLE_LEN + np + 1), ESP_ERR_INVALID_ARG, TAG, "DHT marker data underflow after parsing huffcode: %"PRIu32, num_left); num_left -= (1 + JPEG_HUFFMAN_BITS_LEN_TABLE_LEN + np); } @@ -189,11 +245,14 @@ esp_err_t jpeg_parse_dht_marker(jpeg_dec_header_info_t *header_info) esp_err_t jpeg_parse_dri_marker(jpeg_dec_header_info_t *header_info) { + // Guarantee the 2-byte length field is in-buffer before reading it. + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "DRI marker truncated: missing length"); uint16_t lr = jpeg_get_bytes(header_info, 2); if (lr != 4) { ESP_LOGE(TAG, "DRI marker got but stream length is insufficient, the length you got is %" PRIu16, lr); return ESP_ERR_INVALID_SIZE; } + ESP_RETURN_ON_FALSE(header_info->buffer_left >= 2, ESP_ERR_INVALID_ARG, TAG, "DRI marker truncated, buffer_left=%"PRIu32, header_info->buffer_left); header_info->ri = jpeg_get_bytes(header_info, 2); return ESP_OK; } diff --git a/components/esp_driver_jpeg/jpeg_private.h b/components/esp_driver_jpeg/jpeg_private.h index 741c380164c..3a038ce321b 100644 --- a/components/esp_driver_jpeg/jpeg_private.h +++ b/components/esp_driver_jpeg/jpeg_private.h @@ -82,6 +82,7 @@ typedef struct { uint8_t mcux; // the best value of minimum coding unit horizontal unit uint8_t mcuy; // minimum coding unit vertical unit uint8_t qt_tbl_num; // quantization table number + uint8_t qt_tbl_seen_mask; // bit i set => qt_tbl[i] populated by a DQT entry uint32_t qt_tbl[JPEG_COMPONENT_NUMBER_MAX][JPEG_QUANTIZATION_TABLE_LEN]; // quantization table content [id] uint8_t nf; // number of frames uint8_t ci[JPEG_COMPONENT_NUMBER_MAX]; // Component identifier. 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 f8a28fddd99..4b0e179fef1 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 @@ -139,3 +139,59 @@ TEST_CASE("JPEG decode image without Huffman table JPEG->RGB picture", "[jpeg]") free(tx_buf_no_huff); TEST_ESP_OK(jpeg_del_decoder_engine(jpgd_handle)); } + +// Malformed JPEG used as a regression guard for the DQT index OOB write. +// Layout: SOI, then a DQT segment whose table id (Tq) is 4 (> 3). A +// non-hardened parser would index qt_tbl[4] and smash 256 bytes of stack; +// the hardened parser must reject it and return an error without crashing. +static const uint8_t s_malformed_dqt_jpg[] = { + 0xFF, 0xD8, // SOI + 0xFF, 0xDB, // DQT + 0x00, 0x43, // Lq = 67 (2 + 1 id + 64 table bytes) + 0x04, // Pq=0, Tq=4 -> out-of-range table id + // 64 quantization-table bytes (content irrelevant; never consumed + // because the id is rejected first, but kept so the segment length + // is internally consistent and passes the buffer_left check). + [7 ... 70] = 0x01, +}; + +TEST_CASE("JPEG decode rejects malformed DQT index without crashing", "[jpeg]") +{ + jpeg_decoder_handle_t jpgd_handle; + + jpeg_decode_engine_cfg_t decode_eng_cfg = { + .intr_priority = 0, + .timeout_ms = TIMEOUT_MS, + }; + + 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(64 * 64 * 2, &rx_mem_cfg, &rx_buffer_size); + + size_t tx_buffer_size; + uint8_t *tx_buf = (uint8_t*)jpeg_alloc_decoder_mem(sizeof(s_malformed_dqt_jpg), &tx_mem_cfg, &tx_buffer_size); + memcpy(tx_buf, s_malformed_dqt_jpg, sizeof(s_malformed_dqt_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_dqt_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)); +}