More various fixes (F-*)

src/http/httpd.c

@@ -300,14 +300,6 @@ static int parse_http_request(struct http_client *hc, uint8_t *buf, size_t len)
     struct http_request req;
     struct http_url *url = NULL;
     memset(&req, 0, sizeof(struct http_request));
-    decoded_len = http_url_decode(p, len); /* Decode can be done in place */
-    if (decoded_len < 0) {
-        http_send_response_headers(hc, HTTP_STATUS_BAD_REQUEST,
-                http_status_text(HTTP_STATUS_BAD_REQUEST), "text/plain", 0);
-        return decoded_len;
-    }
-    len = (size_t)decoded_len;
-    end = p + len;
     if (len < 4)
         goto bad_request;
     /* Parse the request line */
@@ -328,6 +320,13 @@ static int parse_http_request(struct http_client *hc, uint8_t *buf, size_t len)
         goto bad_request;
     memcpy(req.path, p, n);
     req.path[n] = '\0';
+    decoded_len = http_url_decode(req.path, n);
+    if (decoded_len < 0)
+        goto bad_request;
+    req.path[decoded_len] = '\0';
+    if (memchr(req.path, '\r', (size_t)decoded_len) ||
+            memchr(req.path, '\n', (size_t)decoded_len))
+        goto bad_request;
     p = q + 1;
     q = strchr(p, '\r');
     if (!q)

src/test/unit/unit.c

@@ -1215,6 +1215,7 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_wrong_type_ignored);
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_ttl_exceeded_syn_sent);
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_port_unreach_syn_sent_closes);
+    tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_port_unreach_bad_seq_ignored);
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_src_ip_mismatch_not_closed);
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_frag_needed_reduces_mss);
     tcase_add_test(tc_core, test_icmp_try_deliver_tcp_error_avail_too_small);
@@ -1288,7 +1289,7 @@ Suite *wolf_suite(void)
 #ifdef IP_MULTICAST
     tcase_add_test(tc_core, test_poll_tx_udp_multicast_arp_skipped_uses_mcast_mac);
 #endif /* IP_MULTICAST */
-    /* --- unit_tests_dhcp_edges.c (43 tests) --- */
+    /* --- unit_tests_dhcp_edges.c (46 tests) --- */
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_zero_lease_noop);
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_null_noop);
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_renew_gt_lease_clamped);
@@ -1306,6 +1307,9 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_core, test_dhcp_msg_type_len_ne_1_not_returned);
     tcase_add_test(tc_core, test_dhcp_msg_type_pad_bytes_skipped);
     tcase_add_test(tc_core, test_dhcp_msg_type_truncated_option_returns_neg1);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_wrong_server_id_rejected);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_absent_server_id_rejected);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_matching_server_id_accepted);
     tcase_add_test(tc_core, test_dhcp_parse_offer_type_ack_not_offer_rejected);
     tcase_add_test(tc_core, test_dhcp_parse_offer_subnet_mask_len_lt4_rejected);
     tcase_add_test(tc_core, test_dhcp_parse_offer_inner_truncated_opt2_rejected);
@@ -1360,7 +1364,8 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_core, test_arp_recv_reply_sender_multicast_rejected);
     tcase_add_test(tc_core, test_arp_recv_reply_sender_own_ip_rejected);
     tcase_add_test(tc_core, test_arp_recv_reply_sender_zero_ip_rejected);
-    tcase_add_test(tc_core, test_arp_recv_valid_request_caches_neighbor_when_pending);
+    tcase_add_test(tc_core, test_arp_recv_request_replies_but_reply_caches_neighbor);
+    tcase_add_test(tc_core, test_arp_recv_forged_request_cannot_poison_pending);
     tcase_add_test(tc_core, test_arp_recv_runt_packet_dropped);
     tcase_add_test(tc_core, test_ip_recv_wrong_version_dropped_v6);
     tcase_add_test(tc_core, test_ip_recv_ihl_too_small_dropped);

src/test/unit/unit_tests_dhcp_edges.c

@@ -371,8 +371,62 @@ START_TEST(test_dhcp_msg_type_truncated_option_returns_neg1)
 }
 END_TEST
 
+START_TEST(test_dhcp_msg_type_nak_wrong_server_id_rejected)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+    uint8_t *p;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK);
+    p = (uint8_t *)msg.options + 3;          /* after MSG_TYPE TLV */
+    append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000002U); /* different server */
+    append_end(&p);
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), -1);
+}
+END_TEST
+
+START_TEST(test_dhcp_msg_type_nak_absent_server_id_rejected)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK); /* no SERVER_ID option */
+    ((uint8_t *)msg.options)[3] = DHCP_OPTION_END;
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), -1);
+}
+END_TEST
+
+START_TEST(test_dhcp_msg_type_nak_matching_server_id_accepted)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+    uint8_t *p;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK);
+    p = (uint8_t *)msg.options + 3;
+    append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); /* committed server */
+    append_end(&p);
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), DHCP_NAK);
+}
+END_TEST
+
 /* -------------------------------------------------------------------------
- * dhcp_parse_offer — additional branches
+ * dhcp_parse_offer - additional branches
  * ---------------------------------------------------------------------- */
 
 START_TEST(test_dhcp_parse_offer_type_ack_not_offer_rejected)

src/test/unit/unit_tests_ip_arp_recv.c

@@ -1175,13 +1175,17 @@ START_TEST(test_arp_recv_reply_sender_zero_ip_rejected)
 END_TEST
 
 /* =========================================================================
- * arp_recv: valid ARP request IS cached when sender IP is legitimate
+ * arp_recv: a valid ARP request is answered but does NOT install a neighbor;
+ *           only a following REPLY populates the cache
  * =========================================================================
- * Positive test: confirms the happy path around the sender validation
- * runs (sip valid → arp_store_neighbor called when pending match exists).
+ * A REQUEST must never learn a new neighbor, even when it matches an
+ * outstanding pending request -- otherwise a spoofed sender IP/MAC can
+ * poison the cache and lock out the genuine reply (CWE-290, see
+ * test_arp_recv_forged_request_cannot_poison_pending). We still reply to the
+ * request, and the genuine reply that follows resolves the pending entry.
  * We use arp_pending_record directly to bypass the arp_request rate-limit.
  */
-START_TEST(test_arp_recv_valid_request_caches_neighbor_when_pending)
+START_TEST(test_arp_recv_request_replies_but_reply_caches_neighbor)
 {
     struct wolfIP s;
     struct arp_packet arp;
@@ -1198,8 +1202,9 @@ START_TEST(test_arp_recv_valid_request_caches_neighbor_when_pending)
     ll   = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF);
     conf = wolfIP_ipconf_at(&s, TEST_PRIMARY_IF);
 
-    /* Pre-register a pending ARP request so learn path is triggered.
-     * arp_pending_record is the low-level helper; we use it directly. */
+    /* Pre-register a pending ARP request: even so, the request must not be
+     * allowed to learn the sender. arp_pending_record is the low-level
+     * helper; we use it directly. */
     arp_pending_record(&s, TEST_PRIMARY_IF, sender_ip);
 
     build_valid_arp_request(&arp, ll, conf->ip, sender_ip);
@@ -1208,13 +1213,94 @@ START_TEST(test_arp_recv_valid_request_caches_neighbor_when_pending)
     last_frame_sent_size = 0;
     arp_recv(&s, TEST_PRIMARY_IF, &arp, sizeof(arp));
 
-    /* A reply should have been sent */
+    /* A reply should have been sent ... */
     ck_assert_uint_gt(last_frame_sent_size, 0);
-    /* Neighbor must now be in the cache */
+    /* ... but the request must NOT have installed a neighbor entry. */
+    ck_assert_int_lt(arp_neighbor_index(&s, TEST_PRIMARY_IF, sender_ip), 0);
+
+    /* The genuine reply for the still-pending request now populates it. */
+    build_valid_arp_request(&arp, ll, conf->ip, sender_ip);
+    arp.opcode = ee16(ARP_REPLY);
+    memcpy(arp.sma, sender_mac, 6);
+    memcpy(arp.tma, ll->mac, 6);
+    arp.tip = ee32(conf->ip);
+    arp_recv(&s, TEST_PRIMARY_IF, &arp, sizeof(arp));
+
     ck_assert_int_ge(arp_neighbor_index(&s, TEST_PRIMARY_IF, sender_ip), 0);
 }
 END_TEST
 
+/*
+ * The neighbor cache for target_ip must end up holding the genuine MAC,
+ * never the attacker's.  This test FAILS against the vulnerable code.
+ */
+START_TEST(test_arp_recv_forged_request_cannot_poison_pending)
+{
+    struct wolfIP s;
+    struct arp_packet arp;
+    struct wolfIP_ll_dev *ll;
+    struct ipconf *conf;
+    const ip4 our_ip    = 0x0A000001U;
+    const ip4 target_ip = 0x0A000050U;   /* peer the victim is resolving */
+    static const uint8_t attacker_mac[6] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01};
+    static const uint8_t legit_mac[6]    = {0x02, 0xCA, 0xFE, 0xBA, 0xBE, 0x01};
+    uint8_t mac_out[6];
+
+    wolfIP_init(&s);
+    mock_link_init(&s);
+    wolfIP_ipconfig_set(&s, our_ip, 0xFFFFFF00U, 0);
+    wolfIP_filter_set_callback(NULL, NULL);
+    wolfIP_filter_set_mask(0);
+
+    ll   = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF);
+    conf = wolfIP_ipconf_at(&s, TEST_PRIMARY_IF);
+    s.last_tick = 1000;
+
+    /* (1) Victim issues an outbound ARP request for target_ip. */
+    arp_pending_record(&s, TEST_PRIMARY_IF, target_ip);
+
+    /* (2) Attacker forges a REQUEST: sender IP is the peer being resolved,
+     *     but the sender MAC is the attacker's. */
+    memset(&arp, 0, sizeof(arp));
+    memcpy(arp.eth.dst, ll->mac, 6);
+    memcpy(arp.eth.src, attacker_mac, 6);
+    arp.eth.type = ee16(ETH_TYPE_ARP);
+    arp.htype    = ee16(1);
+    arp.ptype    = ee16(0x0800);
+    arp.hlen     = 6;
+    arp.plen     = 4;
+    arp.opcode   = ee16(ARP_REQUEST);
+    memcpy(arp.sma, attacker_mac, 6);
+    arp.sip      = ee32(target_ip);
+    memset(arp.tma, 0, 6);
+    arp.tip      = ee32(conf->ip);
+    s.last_tick += 1;
+    arp_recv(&s, TEST_PRIMARY_IF, &arp, sizeof(arp));
+
+    /* (3) The genuine reply from target_ip arrives with the real MAC. */
+    memset(&arp, 0, sizeof(arp));
+    memcpy(arp.eth.dst, ll->mac, 6);
+    memcpy(arp.eth.src, legit_mac, 6);
+    arp.eth.type = ee16(ETH_TYPE_ARP);
+    arp.htype    = ee16(1);
+    arp.ptype    = ee16(0x0800);
+    arp.hlen     = 6;
+    arp.plen     = 4;
+    arp.opcode   = ee16(ARP_REPLY);
+    memcpy(arp.sma, legit_mac, 6);
+    arp.sip      = ee32(target_ip);
+    memcpy(arp.tma, ll->mac, 6);
+    arp.tip      = ee32(conf->ip);
+    s.last_tick += 1;
+    arp_recv(&s, TEST_PRIMARY_IF, &arp, sizeof(arp));
+
+    /* The genuine MAC must win: a forged request must never install (and
+     * then lock in) a spoofed MAC for a peer the victim is resolving. */
+    ck_assert_int_eq(arp_lookup(&s, TEST_PRIMARY_IF, target_ip, mac_out), 0);
+    ck_assert_mem_eq(mac_out, legit_mac, 6);
+}
+END_TEST
+
 /* =========================================================================
  * arp_recv: runt packet (too short) — dropped
  * =========================================================================

src/test/unit/unit_tests_proto.c

@@ -2694,8 +2694,9 @@ START_TEST(test_arp_request_handling) {
     memcpy(arp_req.sma, req_mac, 6);
     arp_req.tip = ee32(device_ip);
 
-    /* Model a solicited learn: stack has an outstanding ARP request for
-     * req_ip, so the request handler is allowed to populate the cache. */
+    /* Even with an outstanding ARP request for req_ip, the request handler
+     * must not learn the sender: only a REPLY may populate the cache,
+     * otherwise a spoofed sender IP/MAC could poison it (CWE-290). */
     s.last_tick = 1000;
     arp_pending_record(&s, TEST_PRIMARY_IF, req_ip);
 
@@ -2705,9 +2706,8 @@ START_TEST(test_arp_request_handling) {
     wolfIP_poll(&s, 1001);
     wolfIP_poll(&s, 1002);
 
-    /* Check if ARP table updated with requester's MAC and IP */
-    ck_assert_int_eq(arp_lookup(&s, TEST_PRIMARY_IF, req_ip, mac), 0);
-    ck_assert_mem_eq(mac, req_mac, 6);
+    /* ARP table must NOT have been populated from the request */
+    ck_assert_int_eq(arp_lookup(&s, TEST_PRIMARY_IF, req_ip, mac), -1);
 
     /* Check if an ARP reply was generated */
     arp_reply = (struct arp_packet *)last_frame_sent;

src/test/unit/unit_tests_tcp_ack.c

@@ -2321,16 +2321,16 @@ START_TEST(test_arp_recv_request_does_not_store_self_neighbor)
     for (i = 0; i < MAX_NEIGHBORS; i++) {
         if (s.arp.neighbors[i].if_idx != TEST_PRIMARY_IF)
             continue;
-        if (s.arp.neighbors[i].ip == sender_ip) {
+        if (s.arp.neighbors[i].ip == sender_ip)
             sender_count++;
-            ck_assert_mem_eq(s.arp.neighbors[i].mac, sender_mac,
-                             sizeof(sender_mac));
-        }
         if (s.arp.neighbors[i].ip == local_ip)
             self_count++;
     }
 
-    ck_assert_int_eq(sender_count, 1);
+    /* A REQUEST must never populate the cache: not our own IP (self), and
+     * -- even with an outstanding pending request -- not the sender either,
+     * since a spoofed sender IP/MAC could otherwise poison it (CWE-290). */
+    ck_assert_int_eq(sender_count, 0);
     ck_assert_int_eq(self_count, 0);
 }
 END_TEST

src/test/unit/unit_tests_tcp_state.c

@@ -1645,7 +1645,9 @@ START_TEST(test_icmp_try_deliver_tcp_error_port_unreach_syn_sent_closes)
     memset(ts, 0, sizeof(*ts));
     ts->proto = WI_IPPROTO_TCP;
     ts->S = &s;
-    ts->sock.tcp.state = TCP_SYN_SENT;
+    ts->sock.tcp.state   = TCP_SYN_SENT;
+    ts->sock.tcp.seq     = 0xDEADBEEFU;   /* ISS */
+    ts->sock.tcp.snd_una = 0xDEADBEEFU;
     ts->local_ip  = 0x0A000001U;
     ts->remote_ip = 0x0A000064U;
     ts->src_port  = 54321;
@@ -1668,14 +1670,76 @@ START_TEST(test_icmp_try_deliver_tcp_error_port_unreach_syn_sent_closes)
     dp = ee16(ts->dst_port);
     memcpy(orig_tcp_hdr,     &sp, 2);
     memcpy(orig_tcp_hdr + 2, &dp, 2);
+    /* RFC 5927 4.1: embedded SEG.SEQ is the in-flight SYN (== ISS == snd_una),
+     * so it falls inside the [snd_una, snd_una+1) send window. */
+    {
+        uint32_t emb_seq = ee32(ts->sock.tcp.seq);
+        memcpy(orig_tcp_hdr + 4, &emb_seq, 4);
+    }
 
     icmp_try_deliver_tcp_error(&s, icmp);
 
-    /* PORT_UNREACH on SYN_SENT must close the socket */
+    /* PORT_UNREACH on SYN_SENT with an in-window seq must close the socket */
     ck_assert_int_eq(ts->proto, 0);
 }
 END_TEST
 
+START_TEST(test_icmp_try_deliver_tcp_error_port_unreach_bad_seq_ignored)
+{
+    struct wolfIP s;
+    struct tsocket *ts;
+    uint8_t buf[sizeof(struct wolfIP_icmp_packet) + IP_HEADER_LEN + 8];
+    struct wolfIP_icmp_packet *icmp = (struct wolfIP_icmp_packet *)buf;
+    struct wolfIP_ip_wire *orig_ip;
+    uint8_t *orig_tcp_hdr;
+    uint16_t sp, dp;
+    uint32_t bad_seq;
+
+    wolfIP_init(&s);
+    mock_link_init(&s);
+    wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0);
+
+    ts = &s.tcpsockets[0];
+    memset(ts, 0, sizeof(*ts));
+    ts->proto = WI_IPPROTO_TCP;
+    ts->S = &s;
+    ts->sock.tcp.state   = TCP_SYN_SENT;
+    ts->sock.tcp.seq     = 0xDEADBEEFU;   /* ISS */
+    ts->sock.tcp.snd_una = 0xDEADBEEFU;
+    ts->local_ip  = 0x0A000001U;
+    ts->remote_ip = 0x0A000064U;
+    ts->src_port  = 54321;
+    ts->dst_port  = 443;
+    fifo_init(&ts->sock.tcp.txbuf, ts->txmem, TXBUF_SIZE);
+
+    memset(buf, 0, sizeof(buf));
+    icmp->ip.len = ee16((uint16_t)(sizeof(buf) - ETH_HEADER_LEN));
+    icmp->type   = ICMP_DEST_UNREACH;
+    icmp->code   = ICMP_PORT_UNREACH;
+
+    orig_ip = (struct wolfIP_ip_wire *)(buf + sizeof(struct wolfIP_icmp_packet));
+    orig_ip->ver_ihl = 0x45;
+    orig_ip->proto   = WI_IPPROTO_TCP;
+    orig_ip->src     = ee32(ts->local_ip);
+    orig_ip->dst     = ee32(ts->remote_ip);
+
+    orig_tcp_hdr = (uint8_t *)orig_ip + IP_HEADER_LEN;
+    sp = ee16(ts->src_port);
+    dp = ee16(ts->dst_port);
+    memcpy(orig_tcp_hdr,     &sp, 2);
+    memcpy(orig_tcp_hdr + 2, &dp, 2);
+
+    /* seq far outside [snd_una, snd_una+1) */
+    bad_seq = ee32(0x41414141U);
+    memcpy(orig_tcp_hdr + 4, &bad_seq, 4);
+
+    icmp_try_deliver_tcp_error(&s, icmp);
+
+    /* Out-of-window seq: socket must NOT be closed */
+    ck_assert_int_ne(ts->proto, 0);
+}
+END_TEST
+
 /* ICMP DEST_UNREACH with mismatched src_ip: socket not closed */
 START_TEST(test_icmp_try_deliver_tcp_error_src_ip_mismatch_not_closed)
 {