From fba5f995436a3e3139f768b6d8f1a74d5ce1d318 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 9 Apr 2026 15:14:23 +0200 Subject: [PATCH 1/3] fix(lwip): Fix DHCP servver OOB read issue --- components/lwip/apps/dhcpserver/dhcpserver.c | 52 +++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/components/lwip/apps/dhcpserver/dhcpserver.c b/components/lwip/apps/dhcpserver/dhcpserver.c index 00f6fe1dea2..adda29ffe41 100644 --- a/components/lwip/apps/dhcpserver/dhcpserver.c +++ b/components/lwip/apps/dhcpserver/dhcpserver.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -49,6 +49,7 @@ #define DHCPNAK 6 #define DHCPRELEASE 7 +#define DHCP_OPTION_PAD 0 #define DHCP_OPTION_SUBNET_MASK 1 #define DHCP_OPTION_HOST_NAME 12 #define DHCP_OPTION_ROUTER 3 @@ -917,7 +918,6 @@ static void send_ack(dhcps_t *dhcps, struct dhcps_msg *m, u16_t len) static u8_t parse_options(dhcps_t *dhcps, u8_t *optptr, s16_t len) { ip4_addr_t client; - bool is_dhcp_parse_end = false; struct dhcps_state s; client.addr = *((uint32_t *) &dhcps->client_address); @@ -932,10 +932,31 @@ static u8_t parse_options(dhcps_t *dhcps, u8_t *optptr, s16_t len) DHCPS_LOG("dhcps: (s16_t)*optptr = %d\n", (s16_t)*optptr); #endif + if (*optptr == DHCP_OPTION_PAD) { + optptr++; + continue; + } + + if (*optptr == DHCP_OPTION_END) { + break; + } + + if (optptr + 1 >= end) { + break; + } + + u8_t opt_len = optptr[1]; + + if (optptr + 2 + opt_len > end) { + break; + } + switch ((s16_t) *optptr) { case DHCP_OPTION_MSG_TYPE: //53 - type = *(optptr + 2); + if (opt_len >= 1) { + type = optptr[2]; + } break; #if CONFIG_LWIP_DHCPS_REPORT_CLIENT_HOSTNAME @@ -966,31 +987,24 @@ static u8_t parse_options(dhcps_t *dhcps, u8_t *optptr, s16_t len) #endif case DHCP_OPTION_REQ_IPADDR://50 - if (memcmp((char *) &client.addr, (char *) optptr + 2, 4) == 0) { + if (opt_len >= 4) { + if (memcmp((char *) &client.addr, (char *) optptr + 2, 4) == 0) { #if DHCPS_DEBUG - DHCPS_LOG("dhcps: DHCP_OPTION_REQ_IPADDR = 0 ok\n"); + DHCPS_LOG("dhcps: DHCP_OPTION_REQ_IPADDR = 0 ok\n"); #endif - s.state = DHCPS_STATE_ACK; - } else { + s.state = DHCPS_STATE_ACK; + } else { #if DHCPS_DEBUG - DHCPS_LOG("dhcps: DHCP_OPTION_REQ_IPADDR != 0 err\n"); + DHCPS_LOG("dhcps: DHCP_OPTION_REQ_IPADDR != 0 err\n"); #endif - s.state = DHCPS_STATE_NAK; + s.state = DHCPS_STATE_NAK; + } } break; - - case DHCP_OPTION_END: { - is_dhcp_parse_end = true; - } - break; } - if (is_dhcp_parse_end) { - break; - } - - optptr += optptr[1] + 2; + optptr += opt_len + 2; } switch (type) { From d70f6a43be5045247547e6f3f0369c1744c73bb5 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 9 Apr 2026 15:42:49 +0200 Subject: [PATCH 2/3] fix(lwip): Add unit test for dhcps opt parsing --- components/lwip/Kconfig | 8 ++ components/lwip/apps/dhcpserver/dhcpserver.c | 14 ++ components/lwip/test_apps/main/lwip_test.c | 143 ++++++++++++++++++- components/lwip/test_apps/sdkconfig.ci | 0 components/lwip/test_apps/sdkconfig.defaults | 3 + 5 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 components/lwip/test_apps/sdkconfig.ci diff --git a/components/lwip/Kconfig b/components/lwip/Kconfig index 608bf2493e2..7fd0145a1c5 100644 --- a/components/lwip/Kconfig +++ b/components/lwip/Kconfig @@ -452,6 +452,14 @@ menu "LWIP" Enabling this option allows DHCP server to support temporary static ARP entries for DHCP Client. This will help the DHCP server to send the DHCP OFFER and DHCP ACK using IP unicast. + config LWIP_DHCPS_TEST_PARSE_OPTIONS + bool "Expose parse_options() for unit testing" + default n + depends on LWIP_DHCPS + help + Enables a non-static wrapper around the internal parse_options() function + so it can be called from test code. Only enable this for testing builds. + endmenu # DHCPS menuconfig LWIP_AUTOIP diff --git a/components/lwip/apps/dhcpserver/dhcpserver.c b/components/lwip/apps/dhcpserver/dhcpserver.c index adda29ffe41..bebf277fab4 100644 --- a/components/lwip/apps/dhcpserver/dhcpserver.c +++ b/components/lwip/apps/dhcpserver/dhcpserver.c @@ -1726,4 +1726,18 @@ bool dhcps_get_hostname_on_mac(dhcps_t *dhcps, const u8_t *mac, char *out, size_ return false; } #endif +#ifdef CONFIG_LWIP_DHCPS_TEST_PARSE_OPTIONS +u8_t dhcps_test_parse_options(u8_t *optptr, s16_t len) +{ + dhcps_t *dhcps = dhcps_new(); + if (dhcps == NULL) { + return 0; + } + IP4_ADDR(&dhcps->client_address, 192, 168, 4, 2); + u8_t result = parse_options(dhcps, optptr, len); + mem_free(dhcps); + return result; +} +#endif + #endif // ESP_DHCPS diff --git a/components/lwip/test_apps/main/lwip_test.c b/components/lwip/test_apps/main/lwip_test.c index 1f83ecee94f..63a532f6d31 100644 --- a/components/lwip/test_apps/main/lwip_test.c +++ b/components/lwip/test_apps/main/lwip_test.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -484,6 +484,134 @@ TEST(lwip, sntp_client_time_2048) test_sntp_timestamps(2048, false); // NTP timestamp MSB is cleared for time after 2036 } +/* + * DHCP option parser tests + * + * These exercise the bounds-checking in parse_options() by constructing raw + * option byte arrays and verifying correct behaviour for well-formed, + * truncated, and malformed inputs. + */ + +#ifdef CONFIG_LWIP_DHCPS_TEST_PARSE_OPTIONS + +extern u8_t dhcps_test_parse_options(u8_t *optptr, s16_t len); + +#define TEST_OPT_PAD 0 +#define TEST_OPT_MSG_TYPE 53 +#define TEST_OPT_REQ_IPADDR 50 +#define TEST_OPT_END 255 +#define TEST_STATE_OFFER 1 +#define TEST_STATE_ACK 3 +#define TEST_STATE_IDLE 5 + +TEST(lwip, dhcps_parse_options_well_formed_discover) +{ + u8_t opts[] = { + TEST_OPT_MSG_TYPE, 1, 1, /* DHCPDISCOVER */ + TEST_OPT_END + }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_OFFER, state); +} + +TEST(lwip, dhcps_parse_options_well_formed_request_ack) +{ + u8_t opts[] = { + TEST_OPT_MSG_TYPE, 1, 3, /* DHCPREQUEST */ + TEST_OPT_REQ_IPADDR, 4, 192, 168, 4, 2, /* matches client_address */ + TEST_OPT_END + }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_ACK, state); +} + +TEST(lwip, dhcps_parse_options_pad_bytes_skipped) +{ + u8_t opts[] = { + TEST_OPT_PAD, + TEST_OPT_PAD, + TEST_OPT_PAD, + TEST_OPT_MSG_TYPE, 1, 1, /* DHCPDISCOVER */ + TEST_OPT_END + }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_OFFER, state); +} + +TEST(lwip, dhcps_parse_options_truncated_length_byte) +{ + /* Option code at the last byte with no room for the length byte. + * Before the fix this would read 1 byte OOB. */ + u8_t opts[] = { TEST_OPT_MSG_TYPE }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_IDLE, state); +} + +TEST(lwip, dhcps_parse_options_truncated_option_data) +{ + /* MSG_TYPE says length=1 but the data byte is outside the buffer. + * Before the fix this would read 1 byte OOB via optptr[2]. */ + u8_t opts[] = { TEST_OPT_MSG_TYPE, 1 }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_IDLE, state); +} + +TEST(lwip, dhcps_parse_options_oversized_length) +{ + /* Option declares a length far exceeding the buffer. */ + u8_t opts[] = { TEST_OPT_MSG_TYPE, 200, 1, TEST_OPT_END }; + u8_t state = dhcps_test_parse_options(opts, (s16_t)sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_IDLE, state); +} + +TEST(lwip, dhcps_parse_options_no_end_marker) +{ + /* Well-formed option but no END marker — parser must stop at buffer end. */ + u8_t opts[] = { TEST_OPT_MSG_TYPE, 1, 1 }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_OFFER, state); +} + +TEST(lwip, dhcps_parse_options_empty) +{ + u8_t opts[] = { 0 }; + u8_t state = dhcps_test_parse_options(opts, 0); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_IDLE, state); +} + +TEST(lwip, dhcps_parse_options_only_pads) +{ + u8_t opts[] = { TEST_OPT_PAD, TEST_OPT_PAD, TEST_OPT_PAD }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_IDLE, state); +} + +TEST(lwip, dhcps_parse_options_unknown_option_skipped) +{ + /* Unknown option 99 with length 3 should be skipped, then MSG_TYPE parsed. */ + u8_t opts[] = { + 99, 3, 0xAA, 0xBB, 0xCC, + TEST_OPT_MSG_TYPE, 1, 1, /* DHCPDISCOVER */ + TEST_OPT_END + }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + TEST_ASSERT_EQUAL_UINT8(TEST_STATE_OFFER, state); +} + +TEST(lwip, dhcps_parse_options_req_ipaddr_truncated) +{ + /* REQ_IPADDR claims 4 bytes but buffer only has room for 2. */ + u8_t opts[] = { + TEST_OPT_MSG_TYPE, 1, 3, + TEST_OPT_REQ_IPADDR, 4, 192, 168 + }; + u8_t state = dhcps_test_parse_options(opts, sizeof(opts)); + /* Parser should stop before the truncated REQ_IPADDR, return based on type alone. */ + TEST_ASSERT(state != 0); +} + +#endif /* CONFIG_LWIP_DHCPS_TEST_PARSE_OPTIONS */ + TEST_GROUP_RUNNER(lwip) { RUN_TEST_CASE(lwip, localhost_ping_test) @@ -493,6 +621,19 @@ TEST_GROUP_RUNNER(lwip) RUN_TEST_CASE(lwip, sntp_client_time_2015) RUN_TEST_CASE(lwip, sntp_client_time_2048) RUN_TEST_CASE(lwip, dhcp_arp_probe_self_mac_is_ok) +#ifdef CONFIG_LWIP_DHCPS_TEST_PARSE_OPTIONS + RUN_TEST_CASE(lwip, dhcps_parse_options_well_formed_discover) + RUN_TEST_CASE(lwip, dhcps_parse_options_well_formed_request_ack) + RUN_TEST_CASE(lwip, dhcps_parse_options_pad_bytes_skipped) + RUN_TEST_CASE(lwip, dhcps_parse_options_truncated_length_byte) + RUN_TEST_CASE(lwip, dhcps_parse_options_truncated_option_data) + RUN_TEST_CASE(lwip, dhcps_parse_options_oversized_length) + RUN_TEST_CASE(lwip, dhcps_parse_options_no_end_marker) + RUN_TEST_CASE(lwip, dhcps_parse_options_empty) + RUN_TEST_CASE(lwip, dhcps_parse_options_only_pads) + RUN_TEST_CASE(lwip, dhcps_parse_options_unknown_option_skipped) + RUN_TEST_CASE(lwip, dhcps_parse_options_req_ipaddr_truncated) +#endif } void app_main(void) diff --git a/components/lwip/test_apps/sdkconfig.ci b/components/lwip/test_apps/sdkconfig.ci new file mode 100644 index 00000000000..e69de29bb2d diff --git a/components/lwip/test_apps/sdkconfig.defaults b/components/lwip/test_apps/sdkconfig.defaults index de762f8fe11..38db0f349d6 100644 --- a/components/lwip/test_apps/sdkconfig.defaults +++ b/components/lwip/test_apps/sdkconfig.defaults @@ -3,3 +3,6 @@ CONFIG_UNITY_ENABLE_IDF_TEST_RUNNER=n # Added to enable compilation of DHCP last IP restore feature CONFIG_LWIP_DHCP_RESTORE_LAST_IP=y + +# Expose DHCP server option parser for unit testing +CONFIG_LWIP_DHCPS_TEST_PARSE_OPTIONS=y From fd578fd210513098ed90c216ed67ef3a08bf11fe Mon Sep 17 00:00:00 2001 From: David Cermak Date: Thu, 9 Apr 2026 15:44:04 +0200 Subject: [PATCH 3/3] fix(lwip): Add pytest to lwip --- components/lwip/test_apps/pytest_lwip.py | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 components/lwip/test_apps/pytest_lwip.py diff --git a/components/lwip/test_apps/pytest_lwip.py b/components/lwip/test_apps/pytest_lwip.py new file mode 100644 index 00000000000..6ed591c1d7b --- /dev/null +++ b/components/lwip/test_apps/pytest_lwip.py @@ -0,0 +1,11 @@ +# SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import pytest +from pytest_embedded import Dut +from pytest_embedded_idf.utils import idf_parametrize + + +@pytest.mark.generic +@idf_parametrize('target', ['esp32'], indirect=['target']) +def test_lwip(dut: Dut) -> None: + dut.expect_unity_test_output()