From 6a93e7ec19ed71948a9cfb741e597a46dc59e429 Mon Sep 17 00:00:00 2001 From: zhangyuyang Date: Sun, 30 Aug 2026 15:44:11 +0800 Subject: [PATCH] [components][sal] Fix SIOCGIFCONF buffer overflow --- components/net/sal/src/sal_socket.c | 35 ++++++++--- components/net/utest/tc_sal_socket.c | 87 ++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/components/net/sal/src/sal_socket.c b/components/net/sal/src/sal_socket.c index b420eb29573f..1369834f7159 100644 --- a/components/net/sal/src/sal_socket.c +++ b/components/net/sal/src/sal_socket.c @@ -1572,21 +1572,40 @@ int sal_ioctlsocket(int socket, long cmd, void *arg) case SIOCGIFCONF: { - struct ifconf *ifconf_tmp; - ifconf_tmp = (struct ifconf *)arg; - int count_size = 0; + const int sal_ifreq_size = (int)sizeof(struct sal_ifreq); + struct ifconf *ifconf_tmp = (struct ifconf *)arg; + char *ifc_buf = ifconf_tmp->ifc_ifcu.ifcu_buf; + int buffer_size = ifconf_tmp->ifc_len; + int copied_size = 0; + + if (buffer_size < 0) + { + LOG_E("ifconfig: network interface device list buffer size error.\n"); + return -1; + } for (node = &(cur_netdev_list->list); node; node = rt_slist_next(node)) { struct sal_ifreq sal_ifreq_temp; - count_size++; + if (ifc_buf == RT_NULL) + { + copied_size += sal_ifreq_size; + continue; + } + + if (buffer_size - copied_size < sal_ifreq_size) + { + break; + } + + rt_memset(&sal_ifreq_temp, 0, sizeof(struct sal_ifreq)); netdev = rt_list_entry(node, struct netdev, list); rt_strcpy(sal_ifreq_temp.ifr_ifrn.ifrn_name, netdev->name); - rt_memcpy(ifconf_tmp->ifc_ifcu.ifcu_buf, &sal_ifreq_temp, sizeof(struct sal_ifreq)); - ifconf_tmp->ifc_ifcu.ifcu_buf += sizeof(struct sal_ifreq); + rt_memcpy(ifc_buf, &sal_ifreq_temp, sizeof(struct sal_ifreq)); + copied_size += sal_ifreq_size; + ifc_buf += sizeof(struct sal_ifreq); } - ifconf_tmp->ifc_len = sizeof(struct sal_ifreq) * count_size; - ifconf_tmp->ifc_ifcu.ifcu_buf = ifconf_tmp->ifc_ifcu.ifcu_buf - sizeof(struct sal_ifreq) * count_size; + ifconf_tmp->ifc_len = copied_size; return 0; } case SIOCGIFINDEX: diff --git a/components/net/utest/tc_sal_socket.c b/components/net/utest/tc_sal_socket.c index db6acd7b7260..4da7701c4b24 100644 --- a/components/net/utest/tc_sal_socket.c +++ b/components/net/utest/tc_sal_socket.c @@ -61,6 +61,16 @@ static char test_send_data[] = "Hello, RT-Thread SAL!"; static char test_recv_buffer[TEST_BUFFER_SIZE]; +struct sal_test_ifconf +{ + int ifc_len; + union + { + char *ifcu_buf; + struct sal_ifreq *ifcu_req; + } ifc_ifcu; +}; + /* Local IP for tests (fallback to loopback) */ static char local_ip[16] = "127.0.0.1"; @@ -164,6 +174,82 @@ static void close_test_socket(int sock) } } +static void TC_sal_socket_siocgifconf(void) +{ + struct sal_test_ifconf ifconf; + struct sal_ifreq *ifreq; + unsigned char buffer[sizeof(struct sal_ifreq)]; + char *original_buffer; + size_t index; + size_t name_length; + int sock; + int ret; + + sock = create_test_socket(AF_INET, SOCK_DGRAM, 0); + uassert_true(sock >= 0); + if (sock < 0) + { + return; + } + + /* A zero-sized destination must not be written. */ + memset(buffer, 0xA5, sizeof(buffer)); + ifconf.ifc_len = 0; + ifconf.ifc_ifcu.ifcu_buf = (char *)buffer; + ret = sal_ioctlsocket(sock, SIOCGIFCONF, &ifconf); + uassert_int_equal(ret, 0); + uassert_int_equal(ifconf.ifc_len, 0); + for (index = 0; index < sizeof(buffer); index++) + { + uassert_int_equal(buffer[index], 0xA5); + } + + /* A NULL destination queries the required buffer size. */ + ifconf.ifc_len = sizeof(buffer); + ifconf.ifc_ifcu.ifcu_buf = RT_NULL; + ret = sal_ioctlsocket(sock, SIOCGIFCONF, &ifconf); + uassert_int_equal(ret, 0); + uassert_true(ifconf.ifc_len >= sizeof(struct sal_ifreq)); + uassert_int_equal(ifconf.ifc_len % sizeof(struct sal_ifreq), 0); + + /* A negative buffer length must be rejected without writing. */ + memset(buffer, 0xA5, sizeof(buffer)); + ifconf.ifc_len = -1; + ifconf.ifc_ifcu.ifcu_buf = (char *)buffer; + ret = sal_ioctlsocket(sock, SIOCGIFCONF, &ifconf); + uassert_int_equal(ret, -1); + for (index = 0; index < sizeof(buffer); index++) + { + uassert_int_equal(buffer[index], 0xA5); + } + + /* Returned bytes outside the interface name must be initialized. */ + memset(buffer, 0xA5, sizeof(buffer)); + original_buffer = (char *)buffer; + ifconf.ifc_len = sizeof(buffer); + ifconf.ifc_ifcu.ifcu_buf = original_buffer; + ret = sal_ioctlsocket(sock, SIOCGIFCONF, &ifconf); + uassert_int_equal(ret, 0); + uassert_int_equal(ifconf.ifc_len, sizeof(struct sal_ifreq)); + uassert_true(ifconf.ifc_ifcu.ifcu_buf == original_buffer); + + ifreq = (struct sal_ifreq *)buffer; + for (name_length = 0; name_length < IFNAMSIZ; name_length++) + { + if (ifreq->ifr_ifrn.ifrn_name[name_length] == '\0') + { + break; + } + } + uassert_true(name_length > 0 && name_length < IFNAMSIZ); + for (index = name_length + 1; index < sizeof(*ifreq); index++) + { + uassert_int_equal(buffer[index], 0); + } + + close_test_socket(sock); +} + /* Server thread function */ static void server_thread_entry(void *parameter) { @@ -1021,6 +1107,7 @@ static void utest_do_tc(void) UTEST_UNIT_RUN(TC_sal_socket_send_recv); UTEST_UNIT_RUN(TC_sal_socket_udp_communication); UTEST_UNIT_RUN(TC_sal_socket_getpeername_getsockname); + UTEST_UNIT_RUN(TC_sal_socket_siocgifconf); UTEST_UNIT_RUN(TC_sal_socket_close); LOG_I("===========================================");