Merge branch 'fix/jpeg_enc_header_oob' into 'master'

fix(jpeg): validate encoder buffer sizes before header/DMA access

See merge request espressif/esp-idf!51900

components/esp_driver_jpeg/jpeg_emit_marker.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <string.h>
 #include "sys/param.h"
+#include "esp_err.h"
 #include "esp_log.h"
 #include "jpeg_private.h"
 #include "private/jpeg_param.h"
@@ -16,10 +17,19 @@
 
 static void emit_byte(jpeg_enc_header_info_t *header_info, uint8_t i)
 {
+    if (header_info->header_len >= header_info->header_buf_size) {
+        header_info->header_buf_overflow = true;
+        return;
+    }
     header_info->header_buf[header_info->header_len] = i;
     header_info->header_len = header_info->header_len + 1;
 }
 
+static esp_err_t emit_check(jpeg_enc_header_info_t *header_info)
+{
+    return header_info->header_buf_overflow ? ESP_ERR_INVALID_ARG : ESP_OK;
+}
+
 static void emit_word(jpeg_enc_header_info_t *header_info, uint16_t i)
 {
     emit_byte(header_info, i >> 8);
@@ -71,7 +81,7 @@ static void compute_quant_table(uint32_t *quant_table, const uint32_t *basic_tab
 esp_err_t emit_soi_marker(jpeg_enc_header_info_t *header_info)
 {
     emit_marker(header_info, JPEG_M_SOI & 0xff);
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_app0_marker(jpeg_enc_header_info_t *header_info)
@@ -98,7 +108,7 @@ esp_err_t emit_app0_marker(jpeg_enc_header_info_t *header_info)
     // No thumbnail image
     emit_byte(header_info, 0);
     emit_byte(header_info, 0);
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_dqt_marker(jpeg_enc_header_info_t *header_info)
@@ -115,7 +125,7 @@ esp_err_t emit_dqt_marker(jpeg_enc_header_info_t *header_info)
         }
     }
 
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_sof_marker(jpeg_enc_header_info_t *header_info)
@@ -175,7 +185,7 @@ esp_err_t emit_sof_marker(jpeg_enc_header_info_t *header_info)
         emit_byte(header_info, (comp_h_samp[i] << 4) + comp_v_samp[i]);
         emit_byte(header_info, (i > 0));
     }
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_dht_marker(jpeg_enc_header_info_t *header_info)
@@ -199,7 +209,7 @@ esp_err_t emit_dht_marker(jpeg_enc_header_info_t *header_info)
         emit_dht(header_info, m_huff_bits[1][1], m_huff_val[1][1], 1, true);
     }
 
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_sos_marker(jpeg_enc_header_info_t *header_info)
@@ -221,7 +231,7 @@ esp_err_t emit_sos_marker(jpeg_enc_header_info_t *header_info)
     emit_byte(header_info, 0); /* spectral selection */
     emit_byte(header_info, 63);
     emit_byte(header_info, 0);
-    return ESP_OK;
+    return emit_check(header_info);
 }
 
 esp_err_t emit_com_marker(jpeg_enc_header_info_t *header_info)
@@ -240,5 +250,5 @@ esp_err_t emit_com_marker(jpeg_enc_header_info_t *header_info)
         emit_byte(header_info, 0);
     }
 
-    return ESP_OK;
+    return emit_check(header_info);
 }

components/esp_driver_jpeg/jpeg_encode.c

@@ -6,6 +6,7 @@
 
 #include <stdio.h>
 #include <string.h>
+#include <inttypes.h>
 #include "esp_heap_caps.h"
 #include "freertos/FreeRTOS.h"
 #include "freertos/queue.h"
@@ -42,6 +43,7 @@ static void s_jpeg_enc_config_picture_color_space(jpeg_encoder_handle_t encoder_
 static void s_jpeg_enc_select_sample_mode(jpeg_encoder_handle_t encoder_engine);
 static void s_encoder_error_log_print(uint32_t status);
 static esp_err_t jpeg_enc_validate_sub_sample(jpeg_enc_src_type_t color_space, jpeg_down_sampling_type_t sub_sample);
+static esp_err_t jpeg_enc_check_inbuf_size(uint32_t width, uint32_t height, uint32_t bit_depth, uint32_t inbuf_size);
 
 static void jpeg_encoder_isr_handle_default(void *arg)
 {
@@ -65,6 +67,7 @@ static void jpeg_encoder_isr_handle_default(void *arg)
 static esp_err_t s_jpeg_set_header_info(jpeg_encoder_handle_t encoder_engine)
 {
     encoder_engine->header_info->header_len = 0;
+    encoder_engine->header_info->header_buf_overflow = false;
     ESP_RETURN_ON_ERROR(emit_soi_marker(encoder_engine->header_info), TAG, "marker emit failed");
     ESP_RETURN_ON_ERROR(emit_app0_marker(encoder_engine->header_info), TAG, "marker emit failed");
     ESP_RETURN_ON_ERROR(emit_dqt_marker(encoder_engine->header_info), TAG, "marker emit failed");
@@ -100,6 +103,19 @@ static esp_err_t jpeg_enc_validate_sub_sample(jpeg_enc_src_type_t color_space, j
     return ESP_OK;
 }
 
+static esp_err_t jpeg_enc_check_inbuf_size(uint32_t width, uint32_t height, uint32_t bit_depth, uint32_t inbuf_size)
+{
+    ESP_RETURN_ON_FALSE(width > 0 && height > 0 && bit_depth > 0, ESP_ERR_INVALID_ARG, TAG, "invalid image geometry");
+    uint64_t required_bits = (uint64_t)width * height * bit_depth;
+    ESP_RETURN_ON_FALSE((required_bits % 8) == 0, ESP_ERR_INVALID_ARG, TAG, "image size is not byte aligned");
+    uint64_t required_size = required_bits / 8;
+    ESP_RETURN_ON_FALSE(required_size <= inbuf_size, ESP_ERR_INVALID_ARG, TAG,
+                        "input buffer is too small for width=%" PRIu32 " height=%" PRIu32 " bit_depth=%" PRIu32
+                        " (need %" PRIu64 ", got %" PRIu32 ")",
+                        width, height, bit_depth, required_size, inbuf_size);
+    return ESP_OK;
+}
+
 esp_err_t jpeg_new_encoder_engine(const jpeg_encode_engine_cfg_t *enc_eng_cfg, jpeg_encoder_handle_t *ret_encoder)
 {
 #if CONFIG_JPEG_ENABLE_DEBUG_LOG
@@ -132,7 +148,7 @@ esp_err_t jpeg_new_encoder_engine(const jpeg_encode_engine_cfg_t *enc_eng_cfg, j
 
     ESP_GOTO_ON_ERROR(jpeg_check_intr_priority(encoder_engine->codec_base, enc_eng_cfg->intr_priority), err, TAG, "set group interrupt priority failed");
     if (enc_eng_cfg->intr_priority) {
-        ESP_RETURN_ON_FALSE(1 << (enc_eng_cfg->intr_priority) & JPEG_ALLOW_INTR_PRIORITY_MASK, ESP_ERR_INVALID_ARG, TAG, "invalid interrupt priority:%d", enc_eng_cfg->intr_priority);
+        ESP_GOTO_ON_FALSE(1 << (enc_eng_cfg->intr_priority) & JPEG_ALLOW_INTR_PRIORITY_MASK, ESP_ERR_INVALID_ARG, err, TAG, "invalid interrupt priority:%d", enc_eng_cfg->intr_priority);
     }
     int isr_flags = JPEG_INTR_ALLOC_FLAG;
     if (enc_eng_cfg->intr_priority) {
@@ -243,9 +259,14 @@ esp_err_t jpeg_encoder_process(jpeg_encoder_handle_t encoder_engine, const jpeg_
     encoder_engine->header_info->origin_h = encode_cfg->width;
     encoder_engine->header_info->origin_v = encode_cfg->height;
     encoder_engine->header_info->header_buf = bit_stream;
+    encoder_engine->header_info->header_buf_size = outbuf_size;
 
     s_jpeg_enc_config_picture_color_space(encoder_engine);
     s_jpeg_enc_select_sample_mode(encoder_engine);
+    /* bytes_per_pixel holds bit depth; reject before programming TX DMA. */
+    ESP_GOTO_ON_ERROR(jpeg_enc_check_inbuf_size(encode_cfg->width, encode_cfg->height,
+                                                encoder_engine->bytes_per_pixel, inbuf_size),
+                      err2, TAG, "input buffer size check failed");
     jpeg_ll_set_picture_height(hal->dev, encoder_engine->header_info->origin_v);
     jpeg_ll_set_picture_width(hal->dev, encoder_engine->header_info->origin_h);
     jpeg_ll_pixel_reverse(hal->dev, encode_cfg->pixel_reverse);
@@ -254,6 +275,7 @@ esp_err_t jpeg_encoder_process(jpeg_encoder_handle_t encoder_engine, const jpeg_
     jpeg_ll_set_qnr_presition(hal->dev, 0);
     ESP_GOTO_ON_ERROR(s_jpeg_set_header_info(encoder_engine), err2, TAG, "set header failed");
     jpeg_hal_set_quantization_coefficient(hal, encoder_engine->header_info->m_quantization_tables[0], encoder_engine->header_info->m_quantization_tables[1]);
+    /* Need at least one byte of payload room after a complete header. */
     ESP_GOTO_ON_FALSE(outbuf_size > encoder_engine->header_info->header_len, ESP_ERR_INVALID_ARG, err2, TAG, "output buffer is too small for jpeg header");
     payload_buf_size = outbuf_size - encoder_engine->header_info->header_len;
 

components/esp_driver_jpeg/jpeg_private.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <stdint.h>
+#include <stdbool.h>
 #include <stdatomic.h>
 #include <sys/queue.h>
 #include "esp_macros.h"
@@ -176,7 +177,9 @@ typedef struct {
 
 typedef struct {
     uint8_t *header_buf;                           // Pointer to the header of jpeg header buffer
+    uint32_t header_buf_size;                      // Capacity of header_buf in bytes
     uint32_t header_len;                           // Record for header length
+    bool header_buf_overflow;                      // Set when emit exceeds header_buf_size
     uint32_t m_quantization_tables[2][JPEG_QUANTIZATION_TABLE_LEN];         // quantization tables
     uint8_t num_components;                        // number of components
     uint32_t origin_h;                             // horizontal of original picture

components/esp_driver_jpeg/test_apps/jpeg_test_apps/main/test_jpeg_encode.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Unlicense OR CC0-1.0
  */
@@ -118,3 +118,60 @@ TEST_CASE("jpeg initialize twice test", "[jpeg]")
     TEST_ESP_OK(jpeg_del_encoder_engine(encoder_handle));
     TEST_ESP_OK(jpeg_del_decoder_engine(decoder_handle));
 }
+
+TEST_CASE("JPEG encode rejects undersized input/output buffers", "[jpeg]")
+{
+    jpeg_encoder_handle_t jpeg_handle = NULL;
+    jpeg_encode_engine_cfg_t encode_eng_cfg = {
+        .timeout_ms = 40,
+    };
+    jpeg_encode_cfg_t enc_config = {
+        .src_type = JPEG_ENCODE_IN_FORMAT_RGB888,
+        .sub_sample = JPEG_DOWN_SAMPLING_YUV422,
+        .image_quality = 80,
+        .width = 640,
+        .height = 480,
+    };
+    jpeg_encode_memory_alloc_cfg_t tx_mem_cfg = {
+        .buffer_direction = JPEG_ENC_ALLOC_INPUT_BUFFER,
+    };
+    jpeg_encode_memory_alloc_cfg_t rx_mem_cfg = {
+        .buffer_direction = JPEG_ENC_ALLOC_OUTPUT_BUFFER,
+    };
+
+    const size_t claimed_inbuf_size = 4096;
+    const size_t out_cap = 64;
+    const size_t canary_len = 256;
+    size_t rgb_file_size = (size_t)image_esp480_rgb_end - (size_t)image_esp480_rgb_start;
+
+    size_t tx_buffer_size = 0;
+    uint8_t *raw_buf = (uint8_t *)jpeg_alloc_encoder_mem(rgb_file_size, &tx_mem_cfg, &tx_buffer_size);
+    TEST_ASSERT_NOT_NULL(raw_buf);
+    memcpy(raw_buf, image_esp480_rgb_start, rgb_file_size);
+
+    size_t rx_buffer_size = 0;
+    uint8_t *out_buf = (uint8_t *)jpeg_alloc_encoder_mem(out_cap + canary_len, &rx_mem_cfg, &rx_buffer_size);
+    TEST_ASSERT_NOT_NULL(out_buf);
+    TEST_ASSERT_TRUE(rx_buffer_size >= out_cap + canary_len);
+    memset(out_buf + out_cap, 0xA5, canary_len);
+
+    TEST_ESP_OK(jpeg_new_encoder_engine(&encode_eng_cfg, &jpeg_handle));
+    uint32_t jpg_size = 0;
+
+    /* Input check runs first; a tiny inbuf_size must not reach header emit / TX DMA. */
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG,
+                 jpeg_encoder_process(jpeg_handle, &enc_config, raw_buf, claimed_inbuf_size,
+                                      out_buf, rx_buffer_size, &jpg_size));
+
+    /* Valid input, claimed output is only 64 bytes; header emit must not write the canary. */
+    TEST_ESP_ERR(ESP_ERR_INVALID_ARG,
+                 jpeg_encoder_process(jpeg_handle, &enc_config, raw_buf, rgb_file_size,
+                                      out_buf, out_cap, &jpg_size));
+    for (size_t i = 0; i < canary_len; i++) {
+        TEST_ASSERT_EQUAL_HEX8(0xA5, out_buf[out_cap + i]);
+    }
+
+    free(out_buf);
+    free(raw_buf);
+    TEST_ESP_OK(jpeg_del_encoder_engine(jpeg_handle));
+}