From 4b960088e5222a1a40ff5a107f3cb9d4e8e48b20 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 5 Feb 2026 12:10:59 +0100 Subject: [PATCH 01/10] validated TCP options length against actual packet size in tcp_process_ts --- src/wolfip.c | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index 721a750..c856a19 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1757,18 +1757,44 @@ static int ip_output_add_header(struct tsocket *t, struct wolfIP_ip_packet *ip, } /* Process timestamp option, calculate RTT */ -static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) +static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, + uint32_t frame_len) { const struct tcp_opt_ts *ts; const uint8_t *opt = tcp->data; - while (opt < ((const uint8_t *)tcp->data + (tcp->hlen >> 2))) { - if (*opt == TCP_OPTION_NOP) + int claimed_opt_len = (tcp->hlen >> 2) - 20; + int available_bytes = (int)(frame_len - sizeof(struct wolfIP_tcp_seg)); + int opt_len; + const uint8_t *opt_end; + + /* Sanity checks */ + if (claimed_opt_len <= 0 || available_bytes <= 0) + return -1; + + /* Use minimum of claimed vs actual available */ + opt_len = (claimed_opt_len < available_bytes) ? claimed_opt_len : available_bytes; + opt_end = opt + opt_len; + + while (opt < opt_end) { + if (*opt == TCP_OPTION_NOP) { opt++; - else if (*opt == TCP_OPTION_EOO) + } else if (*opt == TCP_OPTION_EOO) { break; - else { + } else { + /* Need at least 2 bytes for kind + length */ + if (opt + 2 > opt_end) + break; + ts = (const struct tcp_opt_ts *)opt; + + /* Validate length: minimum 2, must not exceed remaining space */ + if (ts->len < 2 || opt + ts->len > opt_end) + break; + if (ts->opt == TCP_OPTION_TS) { + /* Need full timestamp option (10 bytes) */ + if (ts->len < TCP_OPTION_TS_LEN) + return -1; t->sock.tcp.last_ts = ts->val; if (ts->ecr == 0) return -1; /* No echoed timestamp; fall back to coarse RTT. */ @@ -1854,7 +1880,7 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) if (fresh_desc) { seg = (struct wolfIP_tcp_seg *)(t->txmem + fresh_desc->pos + sizeof(*fresh_desc)); /* Update rtt */ - if (tcp_process_ts(t, seg) < 0) { + if (tcp_process_ts(t, seg, fresh_desc->len) < 0) { /* No timestamp option, use coarse RTT estimation */ int rtt = t->S->last_tick - fresh_desc->time_sent; if (t->sock.tcp.rtt == 0) { @@ -2036,7 +2062,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s t->dst_port = ee16(tcp->src_port); t->remote_ip = ee32(tcp->ip.src); t->events |= CB_EVENT_READABLE; /* Keep flag until application calls accept */ - tcp_process_ts(t, tcp); + tcp_process_ts(t, tcp, frame_len); break; } else if (t->sock.tcp.state == TCP_SYN_SENT) { if (tcp->flags == 0x12) { @@ -2045,7 +2071,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s t->sock.tcp.seq = ee32(tcp->ack); t->sock.tcp.snd_una = t->sock.tcp.seq; t->events |= CB_EVENT_WRITABLE; - tcp_process_ts(t, tcp); + tcp_process_ts(t, tcp, frame_len); tcp_send_ack(t); } } @@ -2081,7 +2107,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s } if (tcp->flags & 0x10) { tcp_ack(t, tcp); - tcp_process_ts(t, tcp); + tcp_process_ts(t, tcp, frame_len); } if (tcplen == 0) return; From 576704de214e814cdd4babab9a615d7fd4ad0a34 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 5 Feb 2026 14:46:26 +0100 Subject: [PATCH 02/10] - fix checksum implicit conversion - validate TCP header langth against IP payload and see if it actually fits or not, return on error --- src/wolfip.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index c856a19..d06e238 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1621,7 +1621,7 @@ static void iphdr_set_checksum(struct wolfIP_ip_packet *ip) while (sum >> 16) { sum = (sum & 0xffff) + (sum >> 16); } - ip->csum = ee16(~sum); + ip->csum = ee16((uint16_t)~sum); } #ifdef ETHERNET @@ -1974,6 +1974,10 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s wolfIP_send_ttl_exceeded(S, if_idx, &tcp->ip); return; } + /* Validate TCP header length fits in IP payload */ + if (iplen < IP_HEADER_LEN + (tcp->hlen >> 2)) { + return; /* malformed: TCP header exceeds IP length */ + } tcplen = iplen - (IP_HEADER_LEN + (tcp->hlen >> 2)); if (tcp->flags & 0x02) { int ws_found = tcp_process_ws(t, tcp); From 80d951f4ac851edbb157b4d74a3124b5961ff1ce Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Thu, 5 Feb 2026 14:52:41 +0100 Subject: [PATCH 03/10] fix misaligned memory access in checksum functions (use memmcpy instead of uint16_t pointer casts) --- src/wolfip.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index d06e238..b241b5e 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1575,48 +1575,54 @@ static uint16_t transport_checksum(union transport_pseudo_header *ph, void *_dat { uint32_t sum = 0; uint32_t i = 0; - uint16_t *ptr = (uint16_t *)ph->buf; - uint16_t *data = (uint16_t *)_data; - uint8_t *data8 = (uint8_t *)_data; + const uint8_t *ptr = (const uint8_t *)ph->buf; + const uint8_t *data = (const uint8_t *)_data; uint16_t len = ee16(ph->ph.len); - for (i = 0; i < 6; i++) { - sum += ee16(ptr[i]); + uint16_t word; + for (i = 0; i < 12; i += 2) { + memcpy(&word, ptr + i, sizeof(word)); + sum += ee16(word); } - for (i = 0; i < (len / 2); i++) { - sum += ee16(data[i]); + for (i = 0; i < (len & ~1u); i += 2) { + memcpy(&word, data + i, sizeof(word)); + sum += ee16(word); } if (len & 0x01) { uint16_t spare = 0; - spare |= (data8[len - 1]) << 8; + spare |= data[len - 1] << 8; sum += spare; } while (sum >> 16) { sum = (sum & 0xffff) + (sum >> 16); } - return ~sum; + return (uint16_t)~sum; } static uint16_t icmp_checksum(struct wolfIP_icmp_packet *icmp, uint16_t len) { uint32_t sum = 0; uint32_t i = 0; - uint16_t *ptr = (uint16_t *)(&icmp->type); - for (i = 0; i < len / 2; i++) { - sum += ee16(ptr[i]); + const uint8_t *ptr = (const uint8_t *)(&icmp->type); + uint16_t word; + for (i = 0; i < (len & ~1u); i += 2) { + memcpy(&word, ptr + i, sizeof(word)); + sum += ee16(word); } while (sum >> 16) { sum = (sum & 0xffff) + (sum >> 16); } - return ~sum; + return (uint16_t)~sum; } static void iphdr_set_checksum(struct wolfIP_ip_packet *ip) { uint32_t sum = 0; uint32_t i = 0; - uint16_t *ptr = (uint16_t *)(&ip->ver_ihl); - for (i = 0; i < IP_HEADER_LEN / 2; i++) { - sum += ee16(ptr[i]); + const uint8_t *ptr = (const uint8_t *)(&ip->ver_ihl); + for (i = 0; i < IP_HEADER_LEN; i += 2) { + uint16_t word; + memcpy(&word, ptr + i, sizeof(word)); + sum += ee16(word); } while (sum >> 16) { sum = (sum & 0xffff) + (sum >> 16); From 7f88f00241b2ca39fe257f853ec4e8bd434f8a25 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 14:19:26 +0100 Subject: [PATCH 04/10] validate TCP window scale options length against actual packet size in tcp_process_ws --- src/wolfip.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index b241b5e..cb8c7c8 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1419,26 +1419,36 @@ static uint16_t tcp_adv_win(const struct tsocket *t) return (uint16_t)win; } -static int tcp_process_ws(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) +static int tcp_process_ws(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, + uint32_t frame_len) { const uint8_t *opt = (const uint8_t *)tcp->data; - uint32_t opt_len = (uint32_t)((tcp->hlen >> 2) - TCP_HEADER_LEN); + int claimed_opt_len = (tcp->hlen >> 2) - TCP_HEADER_LEN; + int available_bytes = (int)(frame_len - sizeof(struct wolfIP_tcp_seg)); + int opt_len; + const uint8_t *opt_end; int found = 0; - while (opt_len > 0) { + + if (claimed_opt_len <= 0 || available_bytes <= 0) + return 0; + + opt_len = (claimed_opt_len < available_bytes) ? claimed_opt_len : available_bytes; + opt_end = opt + opt_len; + + while (opt < opt_end) { if (*opt == TCP_OPTION_NOP) { opt++; - opt_len--; continue; } else if (*opt == TCP_OPTION_EOO) { break; } - if (opt_len < 2) + if (opt + 2 > opt_end) break; - if (opt[1] < 2 || opt[1] > opt_len) + if (opt[1] < 2 || opt + opt[1] > opt_end) break; if (*opt == TCP_OPTION_WS && opt[1] == TCP_OPTION_WS_LEN) { uint8_t shift; - if (opt_len < TCP_OPTION_WS_LEN) + if (opt + TCP_OPTION_WS_LEN > opt_end) break; shift = opt[2]; if (shift > 14) @@ -1446,7 +1456,6 @@ static int tcp_process_ws(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) t->sock.tcp.snd_wscale = shift; found = 1; } - opt_len -= opt[1]; opt += opt[1]; } return found; @@ -1986,7 +1995,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s } tcplen = iplen - (IP_HEADER_LEN + (tcp->hlen >> 2)); if (tcp->flags & 0x02) { - int ws_found = tcp_process_ws(t, tcp); + int ws_found = tcp_process_ws(t, tcp, frame_len); /* Window scale is negotiated only during SYN/SYN-ACK. */ if (t->sock.tcp.state == TCP_LISTEN) { /* Server side: enable if peer offered WS. */ From d48a22e1582c34e45f320aeceacdede75ac3a231 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 14:25:36 +0100 Subject: [PATCH 05/10] guard with tcp_seq_leq to ensure ack is ahead before computing the delta. --- src/wolfip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index cb8c7c8..e45a4f1 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1873,7 +1873,8 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) break; } } - if (t->sock.tcp.snd_una != ack) { + if (t->sock.tcp.snd_una != ack && + tcp_seq_leq(t->sock.tcp.snd_una, ack)) { uint32_t delta = ack - t->sock.tcp.snd_una; if (delta >= t->sock.tcp.bytes_in_flight) t->sock.tcp.bytes_in_flight = 0; From c76f577b74a1af3521b0918bab97a63fb06d4e8f Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 14:30:25 +0100 Subject: [PATCH 06/10] validate ACK range in tcp_ack to reject stale and unsent ssequence numbers, preventing unsigned wraparound in bytes_in_flight --- src/wolfip.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index e45a4f1..6c91b3e 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1874,7 +1874,8 @@ static void tcp_ack(struct tsocket *t, const struct wolfIP_tcp_seg *tcp) } } if (t->sock.tcp.snd_una != ack && - tcp_seq_leq(t->sock.tcp.snd_una, ack)) { + tcp_seq_leq(t->sock.tcp.snd_una, ack) && + tcp_seq_leq(ack, t->sock.tcp.seq)) { uint32_t delta = ack - t->sock.tcp.snd_una; if (delta >= t->sock.tcp.bytes_in_flight) t->sock.tcp.bytes_in_flight = 0; From 7512c64466d0770d520a30a1d91156f7ec635343 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 14:42:14 +0100 Subject: [PATCH 07/10] fix unsigned integer overflows in TCP sequence increment and ICMP checksum update - add tcp_seq_inc() helper for wrap-safe sequence number arithmetic, replacing bare ee32(tcp->seq) + 1 in all five call sites - fold carry in ICMP echo reply checksum update to avoid uint16_t truncation --- src/wolfip.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/wolfip.c b/src/wolfip.c index 6c91b3e..fc16212 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1828,9 +1828,17 @@ static int tcp_process_ts(struct tsocket *t, const struct wolfIP_tcp_seg *tcp, return -1; } +/* Increment a TCP sequence number (wraps at 2^32) */ +static inline uint32_t tcp_seq_inc(uint32_t seq, uint32_t n) +{ + if (n > UINT32_MAX - seq) + return n - (UINT32_MAX - seq) - 1; + return seq + n; +} + #define SEQ_DIFF(a,b) ((a - b) > 0x7FFFFFFF) ? (b - a) : (a - b) -/* Return true if a <= b +/* Return true if a <= b * Take into account wrapping. */ static inline int tcp_seq_leq(uint32_t a, uint32_t b) @@ -2045,7 +2053,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s if (tcp->flags & 0x01) { if (t->sock.tcp.state == TCP_ESTABLISHED) { t->sock.tcp.state = TCP_CLOSE_WAIT; - t->sock.tcp.ack = ee32(tcp->seq) + 1; + t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); tcp_send_ack(t); t->events |= CB_EVENT_CLOSED | CB_EVENT_READABLE; (void)wolfIP_filter_notify_socket_event( @@ -2054,7 +2062,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s } else if (t->sock.tcp.state == TCP_FIN_WAIT_1) { t->sock.tcp.state = TCP_CLOSING; - t->sock.tcp.ack = ee32(tcp->seq) + 1; + t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); tcp_send_ack(t); t->events |= CB_EVENT_CLOSED | CB_EVENT_READABLE; } @@ -2078,7 +2086,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s t->local_ip = syn_dst; t->if_idx = (uint8_t)dst_if; t->sock.tcp.state = TCP_SYN_RCVD; - t->sock.tcp.ack = ee32(tcp->seq) + 1; + t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); t->sock.tcp.seq = wolfIP_getrandom(); t->dst_port = ee16(tcp->src_port); t->remote_ip = ee32(tcp->ip.src); @@ -2088,7 +2096,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s } else if (t->sock.tcp.state == TCP_SYN_SENT) { if (tcp->flags == 0x12) { t->sock.tcp.state = TCP_ESTABLISHED; - t->sock.tcp.ack = ee32(tcp->seq) + 1; + t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); t->sock.tcp.seq = ee32(tcp->ack); t->sock.tcp.snd_una = t->sock.tcp.seq; t->events |= CB_EVENT_WRITABLE; @@ -2122,7 +2130,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s } else if (t->sock.tcp.state == TCP_FIN_WAIT_1) { t->sock.tcp.state = TCP_CLOSING; } - t->sock.tcp.ack = ee32(tcp->seq) + 1; + t->sock.tcp.ack = tcp_seq_inc(ee32(tcp->seq), 1); t->events |= CB_EVENT_CLOSED | CB_EVENT_READABLE; tcp_send_ack(t); } @@ -3075,7 +3083,10 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p } if (!DHCP_IS_RUNNING(s) && (icmp->type == ICMP_ECHO_REQUEST)) { icmp->type = ICMP_ECHO_REPLY; - icmp->csum += 8; + { + uint32_t sum = (uint16_t)icmp->csum + 8; + icmp->csum = (uint16_t)(sum + (sum >> 16)); + } tmp = ip->src; ip->src = ip->dst; ip->dst = tmp; From d902002adf27f91e4acdf3b7fb7887d6cd6e4d25 Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 15:46:55 +0100 Subject: [PATCH 08/10] proper casting in tcp_seq_leq --- src/wolfip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index fc16212..99b9ef6 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -1843,7 +1843,7 @@ static inline uint32_t tcp_seq_inc(uint32_t seq, uint32_t n) */ static inline int tcp_seq_leq(uint32_t a, uint32_t b) { - return (int32_t)(a - b) <= 0; + return ((int32_t)a - (int32_t)b) <= 0; } /* Receive an ack */ From cf401cf9ecc1a370fb37d36e279b0129c5268f5f Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 17:18:34 +0100 Subject: [PATCH 09/10] add missing cast when validating tcp header length that they actually fits in the ip payload --- src/wolfip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfip.c b/src/wolfip.c index 99b9ef6..f9d64a5 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2000,7 +2000,7 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, struct wolfIP_tcp_s return; } /* Validate TCP header length fits in IP payload */ - if (iplen < IP_HEADER_LEN + (tcp->hlen >> 2)) { + if (iplen < (uint32_t)(IP_HEADER_LEN + (tcp->hlen >> 2))) { return; /* malformed: TCP header exceeds IP length */ } tcplen = iplen - (IP_HEADER_LEN + (tcp->hlen >> 2)); From 4bc00982c48c305d71bb0e467b6a0b2949462e6b Mon Sep 17 00:00:00 2001 From: Reda Chouk Date: Fri, 6 Feb 2026 17:22:43 +0100 Subject: [PATCH 10/10] update tcp_process_ts in the unit tests with the new frame_len argument --- src/test/unit/unit.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 90c61c8..b15825e 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -7587,7 +7587,7 @@ START_TEST(test_tcp_process_ts_uses_ecr) opt->eoo = TCP_OPTION_EOO; s.last_tick = 1000; - ck_assert_int_eq(tcp_process_ts(ts, tcp), 0); + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), 0); ck_assert_uint_eq(ts->sock.tcp.rtt, 100); } END_TEST @@ -7618,7 +7618,7 @@ START_TEST(test_tcp_process_ts_nop_then_ts) opt->eoo = TCP_OPTION_EOO; s.last_tick = 120; - ck_assert_int_eq(tcp_process_ts(ts, tcp), 0); + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), 0); } END_TEST @@ -7651,7 +7651,7 @@ START_TEST(test_tcp_process_ts_skips_unknown_option) opt->eoo = TCP_OPTION_EOO; s.last_tick = 250; - ck_assert_int_eq(tcp_process_ts(ts, tcp), 0); + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), 0); } END_TEST @@ -7678,7 +7678,7 @@ START_TEST(test_tcp_process_ts_no_ecr) opt->pad = TCP_OPTION_NOP; opt->eoo = TCP_OPTION_EOO; - ck_assert_int_eq(tcp_process_ts(ts, tcp), -1); + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), -1); } END_TEST @@ -7709,7 +7709,7 @@ START_TEST(test_tcp_process_ts_updates_rtt_when_set) opt->eoo = TCP_OPTION_EOO; s.last_tick = 120; - ck_assert_int_eq(tcp_process_ts(ts, tcp), 0); + ck_assert_int_eq(tcp_process_ts(ts, tcp, sizeof(buf)), 0); ck_assert_uint_ne(ts->sock.tcp.rtt, old_rtt); } END_TEST