- make payload pointer/length calculations in tcp_recv mask

- correct MTU comment

Author: gasbytes

src/test/unit/unit.c

@@ -625,6 +625,7 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_utils, test_tcp_recv_overlapping_ooo_segments_coalesce_on_consume);
     tcase_add_test(tc_utils, test_tcp_input_syn_with_sack_option_enables_sack);
     tcase_add_test(tc_utils, test_tcp_input_ignores_reserved_bits_in_hlen);
+    tcase_add_test(tc_utils, test_tcp_recv_ignores_reserved_bits_in_hlen);
     tcase_add_test(tc_utils, test_tcp_input_syn_with_sack_option_respects_local_sack_offer);
     tcase_add_test(tc_utils, test_tcp_input_iplen_too_big);
     tcase_add_test(tc_utils, test_tcp_checksum_valid_passes);

src/test/unit/unit_tests_tcp_flow.c

@@ -3065,6 +3065,49 @@ START_TEST(test_tcp_recv_queues_payload_and_advances_ack)
 }
 END_TEST
 
+START_TEST(test_tcp_recv_ignores_reserved_bits_in_hlen)
+{
+    /* RFC 9293 3.1: the low nibble of the Data Offset byte is Rsrvd and MUST
+     * be ignored. tcp_recv must therefore base its payload pointer and length
+     * on the masked (high-nibble) header length, otherwise a peer that sets
+     * reserved bits would have its delivered bytes shifted by 4*Rsrvd octets. */
+    struct wolfIP s;
+    struct tsocket *ts;
+    uint8_t payload[8] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h' };
+    uint8_t seg_buf[sizeof(struct wolfIP_tcp_seg) + sizeof(payload)];
+    struct wolfIP_tcp_seg *seg = (struct wolfIP_tcp_seg *)seg_buf;
+    uint32_t seq = 50;
+    uint8_t out[sizeof(payload) + 4];
+    int ret;
+
+    wolfIP_init(&s);
+    ts = &s.tcpsockets[0];
+    memset(ts, 0, sizeof(*ts));
+    ts->proto = WI_IPPROTO_TCP;
+    ts->S = &s;
+    ts->sock.tcp.state = TCP_ESTABLISHED;
+    ts->sock.tcp.ack = seq;
+    ts->sock.tcp.bytes_in_flight = 1;
+    queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, seq);
+
+    memset(seg, 0, sizeof(seg_buf));
+    seg->ip.len = ee16(IP_HEADER_LEN + TCP_HEADER_LEN + sizeof(payload));
+    /* No options (hdr is the bare 20 bytes), but flip every reserved bit on. */
+    seg->hlen = (uint8_t)((TCP_HEADER_LEN << 2) | 0x0F);
+    seg->seq = ee32(seq);
+    seg->flags = (TCP_FLAG_ACK | TCP_FLAG_PSH);
+    memcpy(seg->data, payload, sizeof(payload));
+
+    tcp_recv(ts, seg);
+
+    ck_assert_uint_eq(ts->sock.tcp.ack, seq + sizeof(payload));
+    memset(out, 0, sizeof(out));
+    ret = queue_pop(&ts->sock.tcp.rxbuf, out, sizeof(out));
+    ck_assert_int_eq(ret, (int)sizeof(payload));
+    ck_assert_mem_eq(out, payload, sizeof(payload));
+}
+END_TEST
+
 START_TEST(test_tcp_recv_wrong_state_does_nothing)
 {
     struct wolfIP s;

src/wolfip.c

@@ -2470,8 +2470,8 @@ static void icmp_try_deliver_tcp_error(struct wolfIP *s,
                 next_hop_mtu = ee16(next_hop_mtu);
                 /* RFC 879 / RFC 9293 §3.7.1: IPv4 default MSS is 536; ignore
                  * any PTB whose next-hop MTU cannot accommodate it.  Rejecting
-                 * (rather than clamping) prevents a spoofed ICMP from dragging
-                 * a SYN-negotiated peer_mss down to 536. */
+                 * (rather than clamping) sub-576 MTUs prevents a spoofed ICMP
+                 * from dragging a SYN-negotiated peer_mss below 536. */
                 if (next_hop_mtu >= (IP_HEADER_LEN + TCP_HEADER_LEN + TCP_DEFAULT_MSS)) {
                     uint16_t new_mss =
                         (uint16_t)(next_hop_mtu - (IP_HEADER_LEN + TCP_HEADER_LEN));
@@ -3615,9 +3615,12 @@ static inline uint32_t tcp_seq_inc(uint32_t seq, uint32_t n)
 /* Add a segment to the rx buffer for the application to consume */
 static void tcp_recv(struct tsocket *t, struct wolfIP_tcp_seg *seg)
 {
-    uint32_t seg_len = ee16(seg->ip.len) - (IP_HEADER_LEN + (seg->hlen >> 2));
+    /* RFC 9293 3.1: mask the reserved nibble before deriving header length so a
+     * peer that sets reserved bits cannot shift our payload pointer/length. */
+    uint32_t hdr_len = tcp_data_offset_bytes(seg->hlen);
+    uint32_t seg_len = ee16(seg->ip.len) - (IP_HEADER_LEN + hdr_len);
     uint32_t seq = ee32(seg->seq);
-    const uint8_t *payload = (uint8_t *)seg->ip.data + (seg->hlen >> 2);
+    const uint8_t *payload = (uint8_t *)seg->ip.data + hdr_len;
     if ((t->sock.tcp.state != TCP_ESTABLISHED) &&
         (t->sock.tcp.state != TCP_CLOSE_WAIT) &&
         (t->sock.tcp.state != TCP_FIN_WAIT_1) &&