From a39b7ddbac3f3ce94ee289cace8a7bda609a2c9f Mon Sep 17 00:00:00 2001 From: cpq Date: Fri, 11 Nov 2022 15:03:48 +0000 Subject: [PATCH 1/8] Fix fuzzer use-after-poison READ 1 --- mongoose.c | 2 +- src/mqtt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mongoose.c b/mongoose.c index b9a9d275..c1f4df31 100644 --- a/mongoose.c +++ b/mongoose.c @@ -3196,7 +3196,7 @@ int mg_mqtt_parse(const uint8_t *buf, size_t len, uint8_t version, p += 2; } if (p > end) return MQTT_MALFORMED; - if (version == 5 && p + 1 < end) p += 1 + p[0]; // Skip options + if (version == 5 && p + 2 < end) p += 1 + p[0]; // Skip options if (p > end) return MQTT_MALFORMED; m->data.ptr = (char *) p; m->data.len = (size_t) (end - p); diff --git a/src/mqtt.c b/src/mqtt.c index 0db52256..726cdc41 100644 --- a/src/mqtt.c +++ b/src/mqtt.c @@ -173,7 +173,7 @@ int mg_mqtt_parse(const uint8_t *buf, size_t len, uint8_t version, p += 2; } if (p > end) return MQTT_MALFORMED; - if (version == 5 && p + 1 < end) p += 1 + p[0]; // Skip options + if (version == 5 && p + 2 < end) p += 1 + p[0]; // Skip options if (p > end) return MQTT_MALFORMED; m->data.ptr = (char *) p; m->data.len = (size_t) (end - p); From 7a71038a2755727763895c20b9bba1ad5dd08862 Mon Sep 17 00:00:00 2001 From: jfsimon1981 Date: Fri, 11 Nov 2022 16:09:22 +0100 Subject: [PATCH 2/8] Update mip_test.c --- test/mip_test.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 179 insertions(+), 1 deletion(-) diff --git a/test/mip_test.c b/test/mip_test.c index 4693a3a8..f22a2bdf 100644 --- a/test/mip_test.c +++ b/test/mip_test.c @@ -4,9 +4,25 @@ #define MG_ENABLE_PACKED_FS 0 #include +#include +#include +#include #include "mongoose.c" -#include "driver_mock.c" + #include "driver_mock.c" + +static int s_num_tests = 0; + +#define ASSERT(expr) \ + do { \ + s_num_tests++; \ + if (!(expr)) { \ + printf("FAILURE %s:%d: %s\n", __FILE__, __LINE__, #expr); \ + abort(); \ + } \ + } while (0) + + static void test_queue(void) { static uint8_t @@ -49,9 +65,171 @@ static void test_statechange(void) { onstatechange(&iface); } +// MIP TUNTAP driver +static size_t tap_rx(void *buf, size_t len, void *userdata) { + ssize_t received = read(*(int *) userdata, buf, len); + usleep(1); // This is to avoid 100% CPU + if (received < 0) return 0; + return (size_t) received; +} + +static size_t tap_tx(const void *buf, size_t len, void *userdata) { + ssize_t res = write(*(int *) userdata, buf, len); + if (res < 0) { + MG_ERROR(("tap_tx failed: %d", errno)); + return 0; + } + return (size_t) res; +} + +static bool tap_up(void *userdata) { + return userdata ? true : false; +} + +// HTTP fetches IOs +struct Post_reply { + char* post; // HTTP POST data + void* http_response; // Server response(s) + unsigned int http_responses_received; // Number responses received +}; + +char *fetch(struct mg_mgr *mgr, const char *url, const char *post_data); +static void f_http_fetch_query(struct mg_connection *c, int ev, void *ev_data, void *fn_data); +int get_response_code(char *); // Returns HTTP status code from full char* msg + +static void f_http_fetch_query(struct mg_connection *c, int ev, void *ev_data, void *fn_data) { + static char* http_response = 0; + static bool http_response_allocated = 0; // So that we will update out parameter + unsigned int http_responses_received = 0; + struct Post_reply *post_reply_l; + post_reply_l = (struct Post_reply*)fn_data; + + if (ev == MG_EV_CONNECT) { + mg_printf(c, post_reply_l->post); + } else if (ev == MG_EV_HTTP_MSG) { + struct mg_http_message *hm = (struct mg_http_message *) ev_data; + http_responses_received++; + if (!http_response_allocated) { + http_response = (char*)mg_strdup(hm->message).ptr; + http_response_allocated = 1; + } + if (http_responses_received > 0) { + post_reply_l->http_response = http_response; + post_reply_l->http_responses_received = http_responses_received; + } + } +} + +// Fetch utility returns message from fetch(..., URL, POST) +char *fetch(struct mg_mgr *mgr, const char *url, const char *fn_data) { + struct Post_reply post_reply = {.post=(char*)fn_data, .http_response=0, .http_responses_received=0}; + struct mg_connection *conn; + conn = mg_http_connect(mgr, url, f_http_fetch_query, &post_reply); + ASSERT(conn != NULL); // Assertion on initialisation + for (int i = 0; i < 500 && !post_reply.http_responses_received; i++) { + mg_mgr_poll(mgr, 100); + usleep(10000); // 10 ms. Slow down poll loop to ensure packets transit + } + conn->is_closing = 1; + mg_mgr_poll(mgr, 0); + if (!post_reply.http_responses_received) + return 0; + else + return post_reply.http_response; +} + +// Returns server's HTTP response code +int get_response_code(char * http_msg_raw) { + int http_status = 0; + struct mg_http_message http_msg_parsed; + if (mg_http_parse(http_msg_raw, strlen(http_msg_raw), &http_msg_parsed)) { + http_status = mg_http_status(&http_msg_parsed); + } else { + printf("Error: mg_http_parse()\n"); + ASSERT(http_status != 0); // Couldn't parse. + } + return http_status; +} + +static void test_http_fetch(void) { + // Setup interface + const char *iface = "tap0"; // Network iface + const char *mac = "00:00:01:02:03:78"; // MAC address + int fd = open("/dev/net/tun", O_RDWR); // Open network interface + + struct ifreq ifr; + memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, iface, IFNAMSIZ); + ifr.ifr_flags = IFF_TAP | IFF_NO_PI; + if (ioctl(fd, TUNSETIFF, (void *) &ifr) < 0) { + MG_ERROR(("Failed to setup TAP interface: %s", ifr.ifr_name)); + abort(); // return EXIT_FAILURE; + } + fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); // Non-blocking mode + + MG_INFO(("Opened TAP interface: %s", iface)); + + // Events + struct mg_mgr mgr; // Event manager + mg_mgr_init(&mgr); // Initialise event manager + + // MIP driver + struct mip_driver driver = {.tx = tap_tx, .up = tap_up, .rx = tap_rx}; + struct mip_if mif = {.use_dhcp = true, .driver = &driver, .driver_data = &fd}; + sscanf(mac, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &mif.mac[0], &mif.mac[1], &mif.mac[2], + &mif.mac[3], &mif.mac[4], &mif.mac[5]); + + mip_init(&mgr, &mif); + MG_INFO(("Init done, starting main loop")); + + // DHCP lease + { + if (mif.ip) printf("MIF configuration error: not configured for DHCP\n"); + ASSERT(!mif.ip); // Check we are set for DHCP + int pc = 500; // Timout on DHCP lease 500 ~ approx 5s (typical delay <1s) + while (((pc--)>0)/* & !mif.ip*/) { + mg_mgr_poll(&mgr, 100); + usleep(10000); // 10 ms + } + if (!mif.ip) printf("DHCP lease failed.\n"); + ASSERT(mif.ip); // We have a received lease + } + + // Simple HTTP fetch + { + char* http_feedback = {'\0'}; + const bool ipv6 = 0; + if (ipv6) { + http_feedback = fetch (&mgr, "ipv6.google.com",\ + "GET/ HTTP/1.0\r\nHost: ipv6.google.com\r\n\r\n"); + } else { + http_feedback = fetch (&mgr, "http://cesanta.com",\ + "GET //robots.txt HTTP/1.0\r\nHost: cesanta.com\r\n\r\n"); + } + + ASSERT(*http_feedback != '\0'); // Received HTTP response ? + + int http_status = get_response_code(http_feedback); + // printf("Server response HTTP status code: %d\n",http_status); + ASSERT(http_status != 0); + ASSERT(http_status == 301); // OK: Permanently moved (HTTP->HTTPS redirect) + + if (http_feedback) { + free(http_feedback); + http_feedback = 0; + } + } + + // Clear + mg_mgr_free(&mgr); + ASSERT(mgr.conns == NULL); // Deconstruction OK + close(fd); +} + int main(void) { test_queue(); test_statechange(); + test_http_fetch(); printf("SUCCESS\n"); return 0; } From 71f5be011bebdd472890f036fa1fbaa2c6b69af7 Mon Sep 17 00:00:00 2001 From: "Sergio R. Caprile" Date: Fri, 11 Nov 2022 20:15:01 -0300 Subject: [PATCH 3/8] Update test.yml --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c8a1033e..78d06958 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,6 +114,7 @@ jobs: - path: nxp/nxp-twrkv58f220m-lwip-freertos - path: infineon/infineon-xmc4700_4800-lwip-rtx-rtos - path: ti/ti-ek-tm4c1294xl-http-server + - path: ti/ek-tm4c1294xl-baremetal - path: raspberry/raspberry-pi-pico-w name: ${{ matrix.example.path }} steps: From feee81b7339b660dc130e45468ad87daf559659e Mon Sep 17 00:00:00 2001 From: Jean-Francois Simon Date: Tue, 15 Nov 2022 11:05:55 +0100 Subject: [PATCH 4/8] Adding mip_free() function. --- mip/mip.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mip/mip.c b/mip/mip.c index 84c7f701..b731a4b3 100644 --- a/mip/mip.c +++ b/mip/mip.c @@ -478,7 +478,7 @@ static size_t tx_tcp(struct mip_if *ifp, uint32_t dst_ip, uint8_t flags, struct ip *ip = tx_ip(ifp, 6, ifp->ip, dst_ip, sizeof(struct tcp) + len); struct tcp *tcp = (struct tcp *) (ip + 1); memset(tcp, 0, sizeof(*tcp)); - memmove(tcp + 1, buf, len); + if (buf != NULL && len) memmove(tcp + 1, buf, len); tcp->sport = sport; tcp->dport = dport; tcp->seq = seq; @@ -845,6 +845,11 @@ void mip_init(struct mg_mgr *mgr, struct mip_if *ifp) { } } +void mip_free(struct mip_if * ifp) { + free((char *)ifp->rx.ptr); + free((char *)ifp->tx.ptr); +} + int mg_mkpipe(struct mg_mgr *m, mg_event_handler_t fn, void *d, bool udp) { (void) m, (void) fn, (void) d, (void) udp; MG_ERROR(("Not implemented")); From 6393cd149dc77f8f14f816b3607b0bb25031fceb Mon Sep 17 00:00:00 2001 From: Jean-Francois Simon Date: Tue, 15 Nov 2022 11:06:44 +0100 Subject: [PATCH 5/8] Adding mip_free() function. --- mip/mip.h | 1 + 1 file changed, 1 insertion(+) diff --git a/mip/mip.h b/mip/mip.h index ae932e9a..94a27291 100644 --- a/mip/mip.h +++ b/mip/mip.h @@ -50,6 +50,7 @@ struct mip_if { }; void mip_init(struct mg_mgr *, struct mip_if *); +void mip_free(struct mip_if *); extern struct mip_driver mip_driver_stm32; extern struct mip_driver mip_driver_enc28j60; From 14710b81e43640688a1d5603d6ea3b2d72850947 Mon Sep 17 00:00:00 2001 From: Jean-Francois Simon Date: Tue, 15 Nov 2022 11:07:15 +0100 Subject: [PATCH 6/8] Added MIP free. --- test/mip_test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/mip_test.c b/test/mip_test.c index f22a2bdf..01a0727a 100644 --- a/test/mip_test.c +++ b/test/mip_test.c @@ -197,7 +197,7 @@ static void test_http_fetch(void) { // Simple HTTP fetch { - char* http_feedback = {'\0'}; + char* http_feedback = ""; const bool ipv6 = 0; if (ipv6) { http_feedback = fetch (&mgr, "ipv6.google.com",\ @@ -221,6 +221,7 @@ static void test_http_fetch(void) { } // Clear + mip_free(&mif); mg_mgr_free(&mgr); ASSERT(mgr.conns == NULL); // Deconstruction OK close(fd); From aeab3ef7acb3315c7f356f6a2dc21f80e9341161 Mon Sep 17 00:00:00 2001 From: Jean-Francois Simon Date: Tue, 15 Nov 2022 17:09:29 +0100 Subject: [PATCH 7/8] Fixed C++ builds. --- mongoose.c | 7 ++++++- test/mip_test.c | 25 ++++++++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/mongoose.c b/mongoose.c index c1f4df31..1e7dad37 100644 --- a/mongoose.c +++ b/mongoose.c @@ -7007,7 +7007,7 @@ static size_t tx_tcp(struct mip_if *ifp, uint32_t dst_ip, uint8_t flags, struct ip *ip = tx_ip(ifp, 6, ifp->ip, dst_ip, sizeof(struct tcp) + len); struct tcp *tcp = (struct tcp *) (ip + 1); memset(tcp, 0, sizeof(*tcp)); - memmove(tcp + 1, buf, len); + if (buf != NULL && len) memmove(tcp + 1, buf, len); tcp->sport = sport; tcp->dport = dport; tcp->seq = seq; @@ -7374,6 +7374,11 @@ void mip_init(struct mg_mgr *mgr, struct mip_if *ifp) { } } +void mip_free(struct mip_if * ifp) { + free((char *)ifp->rx.ptr); + free((char *)ifp->tx.ptr); +} + int mg_mkpipe(struct mg_mgr *m, mg_event_handler_t fn, void *d, bool udp) { (void) m, (void) fn, (void) d, (void) udp; MG_ERROR(("Not implemented")); diff --git a/test/mip_test.c b/test/mip_test.c index 01a0727a..790646d8 100644 --- a/test/mip_test.c +++ b/test/mip_test.c @@ -122,7 +122,12 @@ static void f_http_fetch_query(struct mg_connection *c, int ev, void *ev_data, v // Fetch utility returns message from fetch(..., URL, POST) char *fetch(struct mg_mgr *mgr, const char *url, const char *fn_data) { - struct Post_reply post_reply = {.post=(char*)fn_data, .http_response=0, .http_responses_received=0}; + struct Post_reply post_reply; + { + post_reply.post=(char*)fn_data; + post_reply.http_response=0; + post_reply.http_responses_received=0; + } struct mg_connection *conn; conn = mg_http_connect(mgr, url, f_http_fetch_query, &post_reply); ASSERT(conn != NULL); // Assertion on initialisation @@ -135,7 +140,7 @@ char *fetch(struct mg_mgr *mgr, const char *url, const char *fn_data) { if (!post_reply.http_responses_received) return 0; else - return post_reply.http_response; + return (char*)post_reply.http_response; } // Returns server's HTTP response code @@ -174,8 +179,18 @@ static void test_http_fetch(void) { mg_mgr_init(&mgr); // Initialise event manager // MIP driver - struct mip_driver driver = {.tx = tap_tx, .up = tap_up, .rx = tap_rx}; - struct mip_if mif = {.use_dhcp = true, .driver = &driver, .driver_data = &fd}; + struct mip_driver driver; + { + driver.tx = tap_tx; + driver.up = tap_up; + driver.rx = tap_rx; + } + struct mip_if mif; + { + mif.use_dhcp = true; + mif.driver = &driver; + mif.driver_data = &fd; + } sscanf(mac, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &mif.mac[0], &mif.mac[1], &mif.mac[2], &mif.mac[3], &mif.mac[4], &mif.mac[5]); @@ -197,7 +212,7 @@ static void test_http_fetch(void) { // Simple HTTP fetch { - char* http_feedback = ""; + char* http_feedback = (char*)""; const bool ipv6 = 0; if (ipv6) { http_feedback = fetch (&mgr, "ipv6.google.com",\ From 7ba68dd20dc34147cd545479edde909c6c4f8832 Mon Sep 17 00:00:00 2001 From: Jean-Francois Simon Date: Wed, 16 Nov 2022 16:19:50 +0100 Subject: [PATCH 8/8] Proper C/C++ struct initialization. --- mongoose.h | 1 + test/mip_test.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/mongoose.h b/mongoose.h index 9c051df1..6f7dee46 100644 --- a/mongoose.h +++ b/mongoose.h @@ -1467,6 +1467,7 @@ struct mip_if { }; void mip_init(struct mg_mgr *, struct mip_if *); +void mip_free(struct mip_if *); extern struct mip_driver mip_driver_stm32; extern struct mip_driver mip_driver_enc28j60; diff --git a/test/mip_test.c b/test/mip_test.c index 790646d8..fecbb25b 100644 --- a/test/mip_test.c +++ b/test/mip_test.c @@ -179,18 +179,22 @@ static void test_http_fetch(void) { mg_mgr_init(&mgr); // Initialise event manager // MIP driver - struct mip_driver driver; - { + + // Zero init fields required (C/C++ style diverge) + #ifndef __cplusplus + struct mip_driver driver = {.tx = tap_tx, .up = tap_up, .rx = tap_rx}; + struct mip_if mif = {.use_dhcp = true, .driver = &driver, .driver_data = &fd}; + #else + struct mip_driver driver {}; driver.tx = tap_tx; driver.up = tap_up; driver.rx = tap_rx; - } - struct mip_if mif; - { + struct mip_if mif {}; mif.use_dhcp = true; mif.driver = &driver; mif.driver_data = &fd; - } + #endif + sscanf(mac, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &mif.mac[0], &mif.mac[1], &mif.mac[2], &mif.mac[3], &mif.mac[4], &mif.mac[5]);