diff --git a/.github/workflows/wolfip-autocov.yml b/.github/workflows/wolfip-autocov.yml index 6cd0ccf5..b681c7ec 100644 --- a/.github/workflows/wolfip-autocov.yml +++ b/.github/workflows/wolfip-autocov.yml @@ -26,7 +26,7 @@ jobs: - name: Generate coverage JSON run: | - gcovr -r . --exclude "src/test/unit/unit.c" --json -o build/coverage/coverage.json + gcovr -r . --exclude "src/test/unit/unit.c" --gcov-ignore-parse-errors=all --json -o build/coverage/coverage.json - name: Enforce 100% function coverage for src/wolfip.c run: | diff --git a/Makefile b/Makefile index bf5096ba..d3e6dcd1 100644 --- a/Makefile +++ b/Makefile @@ -562,6 +562,7 @@ cov: unit $(COV_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/index.html @$(OPEN_CMD) build/coverage/index.html @@ -576,6 +577,7 @@ autocov: unit $(COV_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/index.html @@ -587,6 +589,7 @@ autocov-multicast: unit-multicast $(COV_MCAST_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/multicast.html @@ -598,6 +601,7 @@ cov-multicast: unit-multicast $(COV_MCAST_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/multicast.html @$(OPEN_CMD) build/coverage/multicast.html @@ -620,6 +624,7 @@ cov-vlan: unit-vlan $(COV_VLAN_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/vlan.html @$(OPEN_CMD) build/coverage/vlan.html @@ -632,6 +637,7 @@ autocov-vlan: unit-vlan $(COV_VLAN_UNIT) @mkdir -p build/coverage @gcovr -r . --exclude "src/test/unit/.*" \ --gcov-ignore-errors=no_working_dir_found \ + --gcov-ignore-parse-errors=all \ --merge-mode-functions=merge-use-line-min \ --html-details -o build/coverage/vlan.html diff --git a/src/http/httpd.c b/src/http/httpd.c index 41e1cb55..bd1aabb0 100644 --- a/src/http/httpd.c +++ b/src/http/httpd.c @@ -429,6 +429,13 @@ static void http_recv_cb(int sd, uint16_t event, void *arg) { hc->ssl = NULL; } wolfIP_sock_close(hc->httpd->ipstack, sd); + /* wolfIP_sock_close on an ESTABLISHED socket only starts the active close + * (FIN_WAIT_1) and returns -EAGAIN; the socket lingers, still carrying this + * callback and its arg. Once we zero client_sd the slot is reused by the + * next accept, so the lingering socket's callback_arg would dangle onto a + * different live connection's state. Deregister it here so a late segment + * on the half-closed socket can no longer fire http_recv_cb. */ + wolfIP_register_callback(hc->httpd->ipstack, sd, NULL, NULL); hc->client_sd = 0; } diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index e8b57e5b..1148d067 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -202,6 +202,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_sock_recvfrom_short_addrlen); tcase_add_test(tc_utils, test_sock_recvfrom_udp_short_addrlen); tcase_add_test(tc_utils, test_sock_recvfrom_icmp_short_addrlen); + tcase_add_test(tc_utils, test_sock_recvfrom_null_buf_rejected); tcase_add_test(tc_utils, test_sock_recvfrom_udp_fifo_alignment); tcase_add_test(tc_utils, test_sock_recvfrom_tcp_close_wait_sets_readable); tcase_add_test(tc_utils, test_sock_recvfrom_udp_readable_stays_when_queue_nonempty); @@ -272,6 +273,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_multicast_udp_send_mac_ttl_loop_and_options); tcase_add_test(tc_utils, test_multicast_igmp_query_refreshes_report); tcase_add_test(tc_utils, test_multicast_igmp_query_bad_checksum_dropped); + tcase_add_test(tc_utils, test_multicast_igmp_query_spoofed_dropped); tcase_add_test(tc_utils, test_multicast_join_requires_configured_ip); tcase_add_test(tc_utils, test_multicast_if_pins_egress_interface); tcase_add_test(tc_utils, test_multicast_loop_does_not_fire_on_blocked_send); @@ -434,6 +436,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_udp_try_recv_full_fifo_drop_does_not_set_readable_or_send_icmp); tcase_add_test(tc_utils, test_dns_callback_bad_flags); tcase_add_test(tc_utils, test_dns_callback_truncated_response_aborts_query); + tcase_add_test(tc_utils, test_dns_inflight_query_state_not_clobbered_by_second_call); tcase_add_test(tc_utils, test_regression_dns_callback_high_bit_octet_ip_no_ub); tcase_add_test(tc_utils, test_dns_callback_bad_name); tcase_add_test(tc_utils, test_dns_callback_short_header_ignored); @@ -644,8 +647,10 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_merge_sack_blocks_wrap_order); tcase_add_test(tc_utils, test_tcp_recv_tracks_holes_and_sack_blocks); tcase_add_test(tc_utils, test_tcp_rebuild_rx_sack_right_edge_wraps); + tcase_add_test(tc_utils, test_tcp_rebuild_rx_sack_triggering_block_first); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_trim_and_promote); tcase_add_test(tc_utils, test_tcp_consume_ooo_wrap_drop_fully_acked); + tcase_add_test(tc_utils, test_tcp_store_ooo_overlap_does_not_exhaust_cache); tcase_add_test(tc_utils, test_tcp_ack_sack_early_retransmit_before_three_dupack); tcase_add_test(tc_utils, test_tcp_input_listen_syn_without_sack_disables_sack); tcase_add_test(tc_utils, test_tcp_input_listen_syn_arms_control_rto); @@ -773,6 +778,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_icmp_try_recv_mismatch_remote_ip); tcase_add_test(tc_proto, test_icmp_try_recv_full_fifo_does_not_signal_readable); tcase_add_test(tc_proto, test_raw_socket_recv_captures_ip_header); + tcase_add_test(tc_proto, test_raw_socket_recv_honors_bound_local_ip_and_if); tcase_add_test(tc_proto, test_raw_socket_send_hdrincl_respected); tcase_add_test(tc_proto, test_raw_socket_send_builds_ip_header); tcase_add_test(tc_proto, test_regression_raw_socket_send_ip_id_network_byte_order); @@ -785,6 +791,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_getsockopt_unsupported_option_returns_einval); tcase_add_test(tc_proto, test_packet_socket_recv_frame); tcase_add_test(tc_proto, test_packet_socket_send_frame); +#if WOLFIP_PACKET_SOCKETS + tcase_add_test(tc_proto, test_packet_socket_tx_filter_block_does_not_resend); +#endif tcase_add_test(tc_proto, test_packet_socket_sendto_wrong_family_returns_einval); tcase_add_test(tc_proto, test_packet_socket_setsockopt_rejected); tcase_add_test(tc_proto, test_packet_socket_getsockopt_rejected); @@ -844,6 +853,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_sock_connect_selects_local_ip_multi_if); tcase_add_test(tc_proto, test_icmp_socket_send_recv); tcase_add_test(tc_proto, test_icmp_input_echo_reply_queues); + tcase_add_test(tc_proto, test_icmp_input_echo_reply_wrong_dst_dropped); tcase_add_test(tc_proto, test_icmp_input_echo_request_reply_sent); tcase_add_test(tc_proto, test_icmp_input_echo_reply_sets_df); tcase_add_test(tc_proto, test_icmp_input_echo_request_bad_checksum_dropped); @@ -918,6 +928,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_transport_checksum); tcase_add_test(tc_utils, test_iphdr_set_checksum); + tcase_add_test(tc_utils, test_iphdr_set_checksum_idempotent_with_stale_csum); tcase_add_test(tc_utils, test_eth_output_add_header); tcase_add_test(tc_utils, test_eth_output_add_header_invalid_if); tcase_add_test(tc_utils, test_ip_output_add_header); @@ -965,6 +976,8 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_sock_bind_icmp_basic_and_rebind); tcase_add_test(tc_core, test_sock_bind_icmp_wrong_family); tcase_add_test(tc_core, test_sock_bind_filter_block_rolls_back); + tcase_add_test(tc_core, test_udp_bind_src_port_deferred_until_filter_approves); + tcase_add_test(tc_core, test_icmp_bind_src_port_deferred_until_filter_approves); tcase_add_test(tc_core, test_sendto_arg_validation); tcase_add_test(tc_core, test_sendto_udp_short_addrlen_and_zero_dest); tcase_add_test(tc_core, test_sendto_udp_auto_assigns_src_port); @@ -1174,6 +1187,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_tcp_parse_options_nop_advances); tcase_add_test(tc_core, test_tcp_parse_options_zero_olen_breaks); tcase_add_test(tc_core, test_tcp_parse_options_timestamp_parsed); + tcase_add_test(tc_core, test_tcp_parse_options_timestamp_overlong_ignored); tcase_add_test(tc_core, test_tcp_parse_options_mss_zero_ignored); tcase_add_test(tc_core, test_tcp_parse_options_sack_permitted_parsed); tcase_add_test(tc_core, test_tcp_input_syn_rcvd_rst_bad_seq_ignored); @@ -1336,6 +1350,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dhcp_timer_cb_bound_lease_not_expired_starts_renew); tcase_add_test(tc_core, test_dhcp_timer_cb_default_state_noop); tcase_add_test(tc_core, test_dhcp_timer_cb_null_arg_noop); + tcase_add_test(tc_core, test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack); + tcase_add_test(tc_core, test_dhcp_parse_ack_without_lease_time_rejected); + tcase_add_test(tc_core, test_dhcp_public_apis_null_stack_safe); /* --- unit_tests_ip_arp_recv.c (34 tests) --- */ tcase_add_test(tc_core, test_ip_recv_limited_broadcast_dst_is_local); tcase_add_test(tc_core, test_ip_recv_directed_broadcast_dst_is_local); @@ -1343,6 +1360,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_ip_recv_forward_arp_hit_sends_immediately); tcase_add_test(tc_core, test_ip_recv_forward_unconfigured_iface_skipped); tcase_add_test(tc_core, test_ip_recv_forward_link_local_src_rpf_drop); + tcase_add_test(tc_core, test_ip_recv_forward_self_ip_src_dropped); tcase_add_test(tc_core, test_ip_recv_options_nop_delivered); tcase_add_test(tc_core, test_ip_recv_options_rr_stripped_and_delivered); tcase_add_test(tc_core, test_ip_recv_options_bad_length_aborts_parse); @@ -1523,6 +1541,11 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_vlan_rx_dei_bit_accepted); tcase_add_test(tc_proto, test_vlan_rx_tagged_arp_processed); tcase_add_test(tc_proto, test_vlan_mtu_inherited); + tcase_add_test(tc_proto, test_vlan_delete_purges_arp_neighbor_cache); +#if WOLFIP_PACKET_SOCKETS + tcase_add_test(tc_proto, test_vlan_packet_socket_parent_and_sub_delivery); + tcase_add_test(tc_proto, test_vlan_packet_socket_wildcard_gets_tagged_once); +#endif #endif /* WOLFIP_VLAN */ suite_add_tcase(s, tc_core); diff --git a/src/test/unit/unit_esp.c b/src/test/unit/unit_esp.c index 1dbe906d..80a6fe86 100644 --- a/src/test/unit/unit_esp.c +++ b/src/test/unit/unit_esp.c @@ -1388,6 +1388,67 @@ START_TEST(test_ip_recv_esp_transport_unwrap_failure_drops_packet) } END_TEST +/* F-5784: an ESP packet whose outer IPv4 header carries options (IHL>5) must + * still be unwrapped and delivered. ip_recv used to dispatch ESP before the + * IP option-strip, so esp_transport_unwrap read the SPI from the option bytes, + * the SA lookup failed, and every IHL>5 ESP packet was silently dropped. */ +START_TEST(test_ip_recv_esp_transport_with_ip_options_delivers_payload) +{ + static uint8_t buf[LINK_MTU + 256]; + struct wolfIP s; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; + struct wolfIP_sockaddr_in sin; + uint8_t payload[] = { 'o', 'p', 't', '!' }; + uint8_t rxbuf[sizeof(payload)] = {0}; + uint32_t frame_len; + uint16_t ip_len; + uint32_t esp_payload_len; + uint8_t *opt; + int udp_sd; + int ret; + + wolfIP_init(&s); + esp_setup(); + esp_add_cbc_test_sas(); + wolfIP_ipconfig_set(&s, atoip4(T_DST), 0xFFFFFF00U, 0); + + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(1234); + sin.sin_addr.s_addr = ee32(atoip4(T_DST)); + ck_assert_int_eq(wolfIP_sock_bind(&s, udp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), 0); + + frame_len = build_udp_ip_packet(buf, sizeof(buf), atoip4(T_SRC), atoip4(T_DST), + 4321, 1234, payload, sizeof(payload)); + ip_len = (uint16_t)(frame_len - ETH_HEADER_LEN); + + ret = esp_transport_wrap(ip, &ip_len); + ck_assert_int_eq(ret, 0); + frame_len = (uint32_t)ip_len + ETH_HEADER_LEN; + + /* Splice a 4-byte Router-Alert option into the outer IP header so it + * becomes IHL=6, shifting the ESP header off its default offset. */ + esp_payload_len = frame_len - (ETH_HEADER_LEN + IP_HEADER_LEN); + opt = buf + ETH_HEADER_LEN + IP_HEADER_LEN; + memmove(opt + 4U, opt, esp_payload_len); + opt[0] = 0x94U; opt[1] = 0x04U; opt[2] = 0x00U; opt[3] = 0x00U; + ip->ver_ihl = 0x46U; + ip->len = ee16((uint16_t)(ee16(ip->len) + 4U)); + ip->csum = 0; + iphdr_set_checksum(ip); + frame_len += 4U; + + ip_recv(&s, 0, ip, frame_len); + + ret = wolfIP_sock_recvfrom(&s, udp_sd, rxbuf, sizeof(rxbuf), 0, NULL, NULL); + ck_assert_int_eq(ret, (int)sizeof(payload)); + ck_assert_mem_eq(rxbuf, payload, sizeof(payload)); +} +END_TEST + /* Mock send that captures the last frame sent. * Used by tests that exercise the full TX path (tcp_send_empty_immediate). */ static uint8_t esp_test_last_frame[LINK_MTU]; @@ -1756,6 +1817,7 @@ static Suite *esp_suite(void) tc = tcase_create("ip_recv"); tcase_add_test(tc, test_ip_recv_esp_transport_delivers_udp_payload); tcase_add_test(tc, test_ip_recv_esp_transport_unwrap_failure_drops_packet); + tcase_add_test(tc, test_ip_recv_esp_transport_with_ip_options_delivers_payload); suite_add_tcase(s, tc); /* No-SA outbound path */ diff --git a/src/test/unit/unit_shared.c b/src/test/unit/unit_shared.c index 21ddd742..2fcda51c 100644 --- a/src/test/unit/unit_shared.c +++ b/src/test/unit/unit_shared.c @@ -139,6 +139,7 @@ static uint16_t socket_cb_last_events; static int timer_cb_calls; static uint32_t dns_lookup_ip; static int dns_lookup_calls; +static int dns_lookup_evil_calls; struct tcp_seg_buf { struct wolfIP_tcp_seg seg; @@ -302,6 +303,12 @@ static void test_dns_lookup_cb(uint32_t ip) dns_lookup_calls++; } +static void test_dns_lookup_evil_cb(uint32_t ip) +{ + (void)ip; + dns_lookup_evil_calls++; +} + static void test_dns_ptr_cb(const char *name) { (void)name; diff --git a/src/test/unit/unit_tests_api.c b/src/test/unit/unit_tests_api.c index 9753e167..360c9d18 100644 --- a/src/test/unit/unit_tests_api.c +++ b/src/test/unit/unit_tests_api.c @@ -1957,6 +1957,14 @@ START_TEST(test_sock_connect_tcp_bound_local_ip_no_match) sin.sin_addr.s_addr = ee32(0x0A000002U); ck_assert_int_eq(wolfIP_sock_connect(&s, tcp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); + /* F-5479: a rejected connect must leave the socket CLOSED, not wedged in + * SYN_SENT. Otherwise the next connect returns a stuck EAGAIN for a SYN + * that was never queued. */ + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSED); + /* Retrying must re-validate (EINVAL again), not report a phantom in-flight + * SYN. */ + ck_assert_int_eq(wolfIP_sock_connect(&s, tcp_sd, (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); + ck_assert_int_eq(ts->sock.tcp.state, TCP_CLOSED); } END_TEST diff --git a/src/test/unit/unit_tests_branches.c b/src/test/unit/unit_tests_branches.c index ad684f72..897b628d 100644 --- a/src/test/unit/unit_tests_branches.c +++ b/src/test/unit/unit_tests_branches.c @@ -569,6 +569,100 @@ START_TEST(test_sock_bind_filter_block_rolls_back) } END_TEST +/* F-5069: the bind callback must not see a committed src_port. If src_port is + * written before WOLFIP_FILT_BINDING runs, a callback that re-enters the stack + * (wolfIP_poll) would have an ingress datagram delivered to the socket while + * the bind is still being vetted (and even if it is rejected). Capture the + * socket's src_port as visible during the callback to prove it is deferred. */ +static struct wolfIP *bind_toctou_stack; +static unsigned int bind_toctou_idx; +static int bind_toctou_is_icmp; +static uint16_t bind_toctou_port_during_cb; +static int bind_toctou_capture_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (event && event->reason == WOLFIP_FILT_BINDING && bind_toctou_stack) { + bind_toctou_port_during_cb = bind_toctou_is_icmp ? + bind_toctou_stack->icmpsockets[bind_toctou_idx].src_port : + bind_toctou_stack->udpsockets[bind_toctou_idx].src_port; + } + return 1; /* reject the bind */ +} + +START_TEST(test_udp_bind_src_port_deferred_until_filter_approves) +{ + struct wolfIP s; + int udp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(udp_sd)]; + + bind_toctou_stack = &s; + bind_toctou_idx = SOCKET_UNMARK(udp_sd); + bind_toctou_is_icmp = 0; + bind_toctou_port_during_cb = 0xFFFF; + wolfIP_filter_set_callback(bind_toctou_capture_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_BINDING)); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(4444); + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, udp_sd, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -1); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + /* The port was not committed (matchable) while the filter was deciding. */ + ck_assert_uint_eq(bind_toctou_port_during_cb, 0); + /* A rejected bind leaves the socket unbound. */ + ck_assert_uint_eq(ts->src_port, 0); +} +END_TEST + +START_TEST(test_icmp_bind_src_port_deferred_until_filter_approves) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + + bind_toctou_stack = &s; + bind_toctou_idx = SOCKET_UNMARK(icmp_sd); + bind_toctou_is_icmp = 1; + bind_toctou_port_during_cb = 0xFFFF; + wolfIP_filter_set_callback(bind_toctou_capture_cb, NULL); + wolfIP_filter_set_mask(WOLFIP_FILT_MASK(WOLFIP_FILT_BINDING)); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_port = ee16(0x4321); /* ICMP echo id */ + sin.sin_addr.s_addr = ee32(0x0A000001U); + ck_assert_int_eq(wolfIP_sock_bind(&s, icmp_sd, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -1); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + ck_assert_uint_eq(bind_toctou_port_during_cb, 0); + ck_assert_uint_eq(ts->src_port, 0); +} +END_TEST + /* ---- wolfIP_sock_sendto extras ---- */ START_TEST(test_sendto_arg_validation) diff --git a/src/test/unit/unit_tests_dhcp_edges.c b/src/test/unit/unit_tests_dhcp_edges.c index 3c3cd68a..f167bb93 100644 --- a/src/test/unit/unit_tests_dhcp_edges.c +++ b/src/test/unit/unit_tests_dhcp_edges.c @@ -1127,3 +1127,124 @@ START_TEST(test_dhcp_timer_cb_null_arg_noop) dhcp_timer_cb(NULL); } END_TEST + +/* F-5698: each DHCP renewal cycle must use a fresh transaction ID. The xid and + * server-id are observable from the initial (broadcast) DORA exchange; if the + * renewal DHCPREQUEST reused that xid, a LAN-adjacent attacker could blindly + * forge a renewal DHCPACK with the captured xid and hijack the gateway without + * ever seeing the (unicast) renewal request. */ +START_TEST(test_dhcp_renew_rerandomizes_xid_rejecting_stale_ack) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + const uint32_t old_xid = 0xDEADBEEFU; /* captured from initial DORA */ + const uint32_t new_xid = 0xC0FFEE11U; /* fresh xid for the renewal */ + const uint32_t server_ip = 0x0A000001U; + const uint32_t good_gw = 0x0A000001U; + const uint32_t attacker_gw = 0x0A0000FEU; + + wolfIP_init(&s); + mock_link_init(&s); + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + primary->ip = 0x0A000064U; + primary->mask = 0xFFFFFF00U; + primary->gw = good_gw; + s.dhcp_server_ip = server_ip; + s.dhcp_ip = primary->ip; + s.dhcp_xid = old_xid; + s.dhcp_state = DHCP_BOUND; + s.dhcp_lease_expires = 0; /* not expired -> renew, do not deconfigure */ + s.dhcp_rebind_at = 2000U; + s.last_tick = 1000U; + s.dhcp_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, + WI_IPPROTO_UDP); + ck_assert_int_gt(s.dhcp_udp_sd, 0); + + /* T1 fires: the renewal cycle must adopt a fresh, unpredictable xid. */ + test_rand_override_enabled = 1; + test_rand_override_value = new_xid; + dhcp_timer_cb(&s); + test_rand_override_enabled = 0; + + ck_assert_int_eq(s.dhcp_state, DHCP_RENEWING); + ck_assert_uint_eq(s.dhcp_xid, new_xid); + ck_assert_uint_ne(s.dhcp_xid, old_xid); + + /* A forged ACK replaying the captured DORA xid must be rejected outright, + * leaving the gateway untouched. */ + build_full_ack(&s, &msg, server_ip, primary->ip, primary->mask, + attacker_gw, 0x08080808U, 120U); + msg.xid = ee32(old_xid); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_uint_eq(primary->gw, good_gw); + + /* The legitimate renewal ACK carrying the new xid is still accepted. */ + build_full_ack(&s, &msg, server_ip, primary->ip, primary->mask, + good_gw, 0x08080808U, 120U); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_uint_eq(primary->gw, good_gw); +} +END_TEST + +/* F-5482: a DHCPACK is only valid with the mandatory IP-address-lease-time + * option (51, RFC 2131). An ACK missing it - or carrying a zero duration - + * must be rejected, never bound, otherwise the lease has no expiry/renewal + * timer and is silently treated as permanent. */ +START_TEST(test_dhcp_parse_ack_without_lease_time_rejected) +{ + struct wolfIP s; + struct dhcp_msg msg; + struct ipconf *primary; + uint8_t *p; + + wolfIP_init(&s); + s.dhcp_xid = 0x5482U; + s.dhcp_server_ip = 0x0A000001U; + s.dhcp_state = DHCP_REQUEST_SENT; + primary = wolfIP_primary_ipconf(&s); + ck_assert_ptr_nonnull(primary); + + /* (1) ACK with server-id/offer-ip/mask/router but NO lease time. */ + build_dhcp_msg_base(&s, &msg, DHCP_ACK); + p = (uint8_t *)msg.options + 3; + append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); + append_opt4(&p, DHCP_OPTION_OFFER_IP, 0x0A000064U); + append_opt4(&p, DHCP_OPTION_SUBNET_MASK, 0xFFFFFF00U); + append_opt4(&p, DHCP_OPTION_ROUTER, 0x0A000001U); + append_end(&p); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_int_ne(s.dhcp_state, DHCP_BOUND); + + /* (2) ACK that carries lease time == 0 is equally invalid. */ + build_dhcp_msg_base(&s, &msg, DHCP_ACK); + p = (uint8_t *)msg.options + 3; + append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); + append_opt4(&p, DHCP_OPTION_OFFER_IP, 0x0A000064U); + append_opt4(&p, DHCP_OPTION_SUBNET_MASK, 0xFFFFFF00U); + append_opt4(&p, DHCP_OPTION_LEASE_TIME, 0U); + append_end(&p); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), -1); + ck_assert_int_ne(s.dhcp_state, DHCP_BOUND); + + /* (3) The same ACK with a valid nonzero lease time binds and schedules a + * lease timer. */ + build_full_ack(&s, &msg, 0x0A000001U, 0x0A000064U, 0xFFFFFF00U, + 0x0A000001U, 0x08080808U, 120U); + s.dhcp_timer = NO_TIMER; + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, sizeof(msg)), 0); + ck_assert_int_eq(s.dhcp_state, DHCP_BOUND); + ck_assert_int_ne(s.dhcp_timer, NO_TIMER); +} +END_TEST + +/* F-5485: the public DHCP helper APIs must tolerate a NULL stack pointer and + * return a deterministic value instead of dereferencing it. */ +START_TEST(test_dhcp_public_apis_null_stack_safe) +{ + ck_assert_int_eq(dhcp_bound(NULL), 0); + ck_assert_int_eq(dhcp_client_is_running(NULL), 0); + ck_assert_int_eq(dhcp_client_init(NULL), -WOLFIP_EINVAL); +} +END_TEST diff --git a/src/test/unit/unit_tests_dns_dhcp.c b/src/test/unit/unit_tests_dns_dhcp.c index 737fcf34..75abea98 100644 --- a/src/test/unit/unit_tests_dns_dhcp.c +++ b/src/test/unit/unit_tests_dns_dhcp.c @@ -71,6 +71,14 @@ static void build_dhcp_ack_msg(struct dhcp_msg *msg, uint32_t server_ip, uint32_ opt->data[2] = (dns_ip >> 8) & 0xFF; opt->data[3] = (dns_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + /* The IP-address-lease-time option (51) is mandatory in a DHCPACK. */ + opt->code = DHCP_OPTION_LEASE_TIME; + opt->len = 4; + opt->data[0] = 0; + opt->data[1] = 0; + opt->data[2] = 0; + opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; } @@ -1580,6 +1588,35 @@ START_TEST(test_sock_recvfrom_icmp_short_addrlen) } END_TEST +/* F-5495: a NULL receive buffer with a nonzero length must be rejected instead + * of being copied into. With a datagram already queued, the prior code reached + * memcpy(buf=NULL, ...) and crashed. */ +START_TEST(test_sock_recvfrom_null_buf_rejected) +{ + struct wolfIP s; + int udp_sd; + struct tsocket *ts; + uint8_t payload[4] = { 1, 2, 3, 4 }; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(udp_sd, 0); + ts = &s.udpsockets[SOCKET_UNMARK(udp_sd)]; + ts->src_port = ee16(4444); + + /* Queue a datagram so the recvfrom path would otherwise reach the copy. */ + enqueue_udp_rx(ts, payload, sizeof(payload), 5555); + + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, udp_sd, NULL, 64, 0, NULL, NULL), + -WOLFIP_EINVAL); + /* Rejected before any dequeue: the datagram is still queued. */ + ck_assert_uint_gt(fifo_len(&ts->sock.udp.rxbuf), 0U); +} +END_TEST + START_TEST(test_sock_recvfrom_udp_fifo_alignment) { struct wolfIP s; @@ -1743,12 +1780,65 @@ START_TEST(test_icmp_input_echo_reply_queues) icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN); + /* The reply is destined to a configured local IP (10.0.0.1). */ + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); ck_assert_ptr_nonnull(fifo_peek(&ts->sock.udp.rxbuf)); ck_assert_uint_eq(ts->last_pkt_ttl, 55); } END_TEST +/* F-5733: icmp_input must only deliver an ECHO_REPLY that is actually addressed + * to one of our configured local IPs, mirroring the ECHO_REQUEST guard. A + * forged reply to a non-local dst (matching only a wildcard local_ip==0 socket + * by a guessed echo id) must be dropped. */ +START_TEST(test_icmp_input_echo_reply_wrong_dst_dropped) +{ + struct wolfIP s; + int icmp_sd; + struct tsocket *ts; + struct wolfIP_icmp_packet icmp; + uint32_t frame_len; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + icmp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_ICMP); + ck_assert_int_gt(icmp_sd, 0); + ts = &s.icmpsockets[SOCKET_UNMARK(icmp_sd)]; + ts->local_ip = 0; /* wildcard: per-socket dst check skipped */ + ts->remote_ip = 0; /* accept from any source */ + ts->src_port = ee16(0x1234); /* echo id the attacker guesses */ + frame_len = (uint32_t)(ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN); + + /* Forged reply addressed to an IP that is NOT one of ours: must be dropped + * before reaching the socket. */ + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A000002U); + icmp.ip.dst = ee32(0x0A000099U); /* not a configured local IP */ + icmp.ip.ttl = 55; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_HEADER_LEN); + icmp.type = ICMP_ECHO_REPLY; + icmp_set_echo_id(&icmp, ts->src_port); + icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + ck_assert_ptr_eq(fifo_peek(&ts->sock.udp.rxbuf), NULL); + + /* The same reply addressed to our configured local IP is delivered. */ + memset(&icmp, 0, sizeof(icmp)); + icmp.ip.src = ee32(0x0A000002U); + icmp.ip.dst = ee32(0x0A000001U); /* configured local IP */ + icmp.ip.ttl = 55; + icmp.ip.len = ee16(IP_HEADER_LEN + ICMP_HEADER_LEN); + icmp.type = ICMP_ECHO_REPLY; + icmp_set_echo_id(&icmp, ts->src_port); + icmp.csum = ee16(icmp_checksum(&icmp, ICMP_HEADER_LEN)); + icmp_input(&s, TEST_PRIMARY_IF, (struct wolfIP_ip_packet *)&icmp, frame_len); + ck_assert_ptr_nonnull(fifo_peek(&ts->sock.udp.rxbuf)); +} +END_TEST + START_TEST(test_icmp_input_echo_request_reply_sent) { struct wolfIP s; @@ -4753,6 +4843,13 @@ START_TEST(test_dhcp_poll_offer_and_ack) opt->data[2] = 0x08; opt->data[3] = 0x08; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0x00; + opt->data[1] = 0x00; + opt->data[2] = 0x00; + opt->data[3] = 0x78; /* 120 s */ + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; @@ -5611,6 +5708,92 @@ START_TEST(test_dns_callback_truncated_response_aborts_query) } END_TEST +/* F-4946: a second DNS API call while a query is in-flight must not touch the + * shared callback/type state. Before the fix, nslookup/wolfIP_dns_ptr_lookup + * overwrote dns_lookup_cb/dns_ptr_cb/dns_query_type before dns_send_query's + * busy-guard ran, so the in-flight query's response invoked the wrong handler + * (A->A hijack) or stalled the resolver (A->PTR flip). */ +START_TEST(test_dns_inflight_query_state_not_clobbered_by_second_call) +{ + struct wolfIP s; + uint16_t id1 = 0, id2 = 0, id3 = 0; + uint8_t response[128]; + int pos; + struct dns_header *hdr = (struct dns_header *)response; + struct dns_question *q; + struct dns_rr *rr; + const uint8_t ip_bytes[4] = {0x01, 0x02, 0x03, 0x04}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x08080808U; + s.last_tick = 100U; + + dns_lookup_calls = 0; + dns_lookup_ip = 0; + dns_lookup_evil_calls = 0; + + /* First lookup goes out: dns_id armed, target callback recorded. */ + ck_assert_int_eq(nslookup(&s, "target.com", &id1, test_dns_lookup_cb), 0); + ck_assert_uint_ne(id1, 0U); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* A->A arm: a second nslookup while in-flight must be rejected as busy + * without replacing the recorded callback. */ + ck_assert_int_eq(nslookup(&s, "evil.com", &id2, test_dns_lookup_evil_cb), -16); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_ptr_eq(s.dns_ptr_cb, NULL); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* A->PTR arm: a PTR lookup while in-flight must not NULL the A callback or + * flip the query type. */ + ck_assert_int_eq(wolfIP_dns_ptr_lookup(&s, 0x08080808U, &id3, + test_dns_ptr_cb), -16); + ck_assert_uint_eq(s.dns_id, id1); + ck_assert_ptr_eq(s.dns_lookup_cb, test_dns_lookup_cb); + ck_assert_ptr_eq(s.dns_ptr_cb, NULL); + ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_A); + + /* The response to the original (still in-flight) query must reach the + * original handler, never the rejected second call's handler. */ + memset(response, 0, sizeof(response)); + hdr->id = ee16(id1); + hdr->flags = ee16(0x8100); + hdr->qdcount = ee16(1); + hdr->ancount = ee16(1); + pos = sizeof(struct dns_header); + response[pos++] = 6; memcpy(&response[pos], "target", 6); pos += 6; + response[pos++] = 3; memcpy(&response[pos], "com", 3); pos += 3; + response[pos++] = 0; + q = (struct dns_question *)(response + pos); + q->qtype = ee16(DNS_A); + q->qclass = ee16(DNS_CLASS_IN); + pos += sizeof(struct dns_question); + response[pos++] = 0xC0; + response[pos++] = (uint8_t)sizeof(struct dns_header); + rr = (struct dns_rr *)(response + pos); + rr->type = ee16(DNS_A); + rr->class = ee16(DNS_CLASS_IN); + rr->ttl = ee32(60); + rr->rdlength = ee16(4); + pos += sizeof(struct dns_rr); + memcpy(&response[pos], ip_bytes, sizeof(ip_bytes)); + pos += sizeof(ip_bytes); + + enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, + (uint16_t)pos, DNS_PORT); + dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s); + + ck_assert_int_eq(dns_lookup_calls, 1); + ck_assert_uint_eq(dns_lookup_ip, 0x01020304U); + ck_assert_int_eq(dns_lookup_evil_calls, 0); + ck_assert_uint_eq(s.dns_id, 0U); +} +END_TEST + START_TEST(test_regression_dns_callback_high_bit_octet_ip_no_ub) { /* The dns_callback() A-record reassembly used to compute diff --git a/src/test/unit/unit_tests_ip_arp_recv.c b/src/test/unit/unit_tests_ip_arp_recv.c index f0407a77..18a2771b 100644 --- a/src/test/unit/unit_tests_ip_arp_recv.c +++ b/src/test/unit/unit_tests_ip_arp_recv.c @@ -321,6 +321,66 @@ START_TEST(test_ip_recv_forward_link_local_src_rpf_drop) } END_TEST +/* ========================================================================= + * F-5697: a packet whose source is the router's OWN IP on the ingress + * interface must never be forwarded. + * ========================================================================= + * The strict-RPF loop skips i == if_idx, so the ingress interface's own + * address was never compared against the source. An L2-adjacent attacker + * could spoof the router's LAN IP and have it forwarded out another + * interface. A legitimate host in the same ingress subnet must still be + * forwarded (the fix only matches the exact own /32, not the subnet). + */ +START_TEST(test_ip_recv_forward_self_ip_src_dropped) +{ + struct wolfIP s; + uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + UDP_HEADER_LEN]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + ip4 primary_ip = 0x0A000001U; /* 10.0.0.1 — router LAN IP (if1) */ + ip4 secondary_ip = 0xC0A80101U; /* 192.168.1.1 — router WAN IP (if2) */ + ip4 dest_ip = 0xC0A80155U; /* 192.168.1.85 — local to if2 */ + static const uint8_t dest_mac[6] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15}; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + wolfIP_filter_set_callback(NULL, NULL); + /* ARP hit so that, absent the drop, the packet would be forwarded now. */ + arp_store_neighbor(&s, TEST_SECOND_IF, dest_ip, dest_mac); + + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, s.ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(ip->eth.src, "\x01\x02\x03\x04\x05\x06", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 64; + ip->proto = WI_IPPROTO_UDP; + ip->len = ee16(IP_HEADER_LEN + UDP_HEADER_LEN); + ip->src = ee32(primary_ip); /* spoof: the router's own LAN IP */ + ip->dst = ee32(dest_ip); + fix_ip_checksum(ip); + { + uint16_t *udp = (uint16_t *)(frame + ETH_HEADER_LEN + IP_HEADER_LEN); + udp[0] = ee16(9999); udp[1] = ee16(53); + udp[2] = ee16(UDP_HEADER_LEN); udp[3] = 0; + } + + last_frame_sent_size = 0; + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + + /* Self-IP source must be dropped: nothing forwarded, nothing queued. */ + ck_assert_uint_eq(last_frame_sent_size, 0); + ck_assert_uint_eq(s.arp_pending[0].dest, IPADDR_ANY); + + /* Sanity: a real host in the same ingress subnet is still forwarded + * (the drop targets the exact own address, not the whole subnet). */ + ip->src = ee32(0x0A000002U); /* 10.0.0.2 — legitimate LAN host */ + fix_ip_checksum(ip); + last_frame_sent_size = 0; + ip_recv(&s, TEST_PRIMARY_IF, ip, (uint32_t)sizeof(frame)); + ck_assert_uint_gt(last_frame_sent_size, 0); + ck_assert_mem_eq(last_frame_sent + 0, dest_mac, 6); +} +END_TEST + /* ========================================================================= * ip_recv: IP with NOP options — options parsed, payload delivered * ========================================================================= diff --git a/src/test/unit/unit_tests_multicast.c b/src/test/unit/unit_tests_multicast.c index 9c93096a..72886c3c 100644 --- a/src/test/unit/unit_tests_multicast.c +++ b/src/test/unit/unit_tests_multicast.c @@ -314,6 +314,94 @@ START_TEST(test_multicast_igmp_query_bad_checksum_dropped) } END_TEST +/* F-5904: igmp_input must apply RFC 3376 §4.1 query validation. A query that + * could not be a legitimate on-link membership query - TTL != 1 (transited a + * router), or a destination that is neither all-hosts (224.0.0.1) nor the + * group - must not solicit membership reports (which would disclose the host's + * group memberships). */ +START_TEST(test_multicast_igmp_query_spoofed_dropped) +{ + struct wolfIP s; + int sd; + struct wolfIP_ip_mreq mreq; + struct wolfIP_ll_dev *ll; + uint8_t frame[ETH_HEADER_LEN + IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN]; + struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)frame; + uint8_t *igmp = frame + ETH_HEADER_LEN + IP_HEADER_LEN; + ip4 group = 0xE9010207U; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000002U, 0xFFFFFF00U, 0); + ll = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(ll); + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(sd, 0); + multicast_mreq(&mreq, group, IPADDR_ANY); + ck_assert_int_eq(wolfIP_sock_setsockopt(&s, sd, WOLFIP_SOL_IP, + WOLFIP_IP_ADD_MEMBERSHIP, &mreq, sizeof(mreq)), 0); + + /* (1) Otherwise-valid general query but with TTL != 1 -> dropped. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, "\x01\x00\x5e\x00\x00\x01", 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 64; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(IGMP_ALL_HOSTS); + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_eq(last_frame_sent_size, 0); + + /* (2) TTL == 1 but addressed to our unicast IP (not all-hosts/group) -> + * dropped. Sent to our unicast MAC so it reaches igmp_input. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, ll->mac, 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 1; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(0x0A000002U); /* our unicast IP */ + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_eq(last_frame_sent_size, 0); + + /* Sanity: a compliant query (TTL 1, all-hosts dst) still solicits a report, + * so the guards did not over-block. */ + memset(frame, 0, sizeof(frame)); + memcpy(ip->eth.dst, "\x01\x00\x5e\x00\x00\x01", 6); + memcpy(ip->eth.src, "\x02\x00\x00\x00\x00\x01", 6); + ip->eth.type = ee16(ETH_TYPE_IP); + ip->ver_ihl = 0x45; + ip->ttl = 1; + ip->proto = WI_IPPROTO_IGMP; + ip->len = ee16(IP_HEADER_LEN + IGMPV3_QUERY_MIN_LEN); + ip->src = ee32(0x0A000001U); + ip->dst = ee32(IGMP_ALL_HOSTS); + igmp[0] = IGMP_TYPE_MEMBERSHIP_QUERY; + put_be32(igmp + 4, group); + put_be16(igmp + 2, ip_checksum_buf(igmp, IGMPV3_QUERY_MIN_LEN)); + fix_ip_checksum(ip); + last_frame_sent_size = 0; + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, sizeof(frame)); + ck_assert_uint_gt(last_frame_sent_size, 0); +} +END_TEST + START_TEST(test_multicast_join_requires_configured_ip) { struct wolfIP s; diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index 15a8942e..404261ab 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4512,6 +4512,33 @@ START_TEST(test_iphdr_set_checksum) { } END_TEST +/* F-5490: iphdr_set_checksum must produce a valid checksum regardless of any + * stale value already in ip->csum, and must be idempotent under repeated calls + * (the setter clears the field itself rather than relying on the caller). */ +START_TEST(test_iphdr_set_checksum_idempotent_with_stale_csum) { + struct wolfIP_ip_packet ip; + memset(&ip, 0, sizeof(ip)); + + ip.ver_ihl = 0x45; + ip.tos = 0; + ip.len = ee16(20); + ip.id = ee16(1); + ip.flags_fo = 0; + ip.ttl = 64; + ip.proto = WI_IPPROTO_TCP; + ip.src = ee32(0xc0a80101); + ip.dst = ee32(0xc0a80102); + ip.csum = ee16(0xBEEF); /* stale, nonzero checksum left by the caller */ + + iphdr_set_checksum(&ip); + ck_assert_int_eq(iphdr_verify_checksum(&ip), 0); + + /* Running it again over the now-populated header must still verify. */ + iphdr_set_checksum(&ip); + ck_assert_int_eq(iphdr_verify_checksum(&ip), 0); +} +END_TEST + // Test for `eth_output_add_header` to add Ethernet headers START_TEST(test_eth_output_add_header) { struct wolfIP_eth_frame eth_frame; @@ -6540,6 +6567,79 @@ START_TEST(test_raw_socket_recv_captures_ip_header) } END_TEST +/* F-5070: raw_try_recv must honour a raw socket's bound_local_ip and if_idx, + * mirroring the TCP/UDP bind contract. A socket bound to one local IP/interface + * must not receive protocol-matching traffic for other destinations or arriving + * on other interfaces. The frame is rebuilt before every injection because the + * unit build enables forwarding, which rewrites the buffer in place. */ +#define RAW_BIND_FRAME_LEN (ETH_HEADER_LEN + IP_HEADER_LEN + 8) +static void raw_bind_build_frame(struct wolfIP *s, struct wolfIP_ip_packet *frame, + size_t bufsz, uint32_t dst) +{ + static const uint8_t payload[8] = {1, 2, 3, 4, 5, 6, 7, 8}; + memset(frame, 0, bufsz); + memcpy(frame->eth.dst, s->ll_dev[TEST_PRIMARY_IF].mac, 6); + memcpy(frame->eth.src, "\xaa\xbb\xcc\xdd\xee\xff", 6); + frame->eth.type = ee16(ETH_TYPE_IP); + frame->ver_ihl = 0x45; + frame->ttl = 32; + frame->proto = WI_IPPROTO_UDP; + frame->len = ee16(IP_HEADER_LEN + (uint16_t)sizeof(payload)); + frame->src = ee32(0x0A000002U); + frame->dst = ee32(dst); + memcpy(frame->data, payload, sizeof(payload)); + iphdr_set_checksum(frame); +} + +START_TEST(test_raw_socket_recv_honors_bound_local_ip_and_if) +{ + struct wolfIP s; + int sd; + struct rawsocket *rs; + uint8_t frame_buf[sizeof(struct wolfIP_ip_packet) + 8]; + struct wolfIP_ip_packet *frame = (struct wolfIP_ip_packet *)frame_buf; + uint8_t rxbuf[64]; + const int delivered = IP_HEADER_LEN + 8; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, WI_IPPROTO_UDP); + ck_assert_int_ge(sd, 0); + rs = &s.rawsockets[SOCKET_UNMARK(sd)]; + + /* Bound to 10.0.0.1: a packet destined elsewhere must be filtered out. */ + rs->bound_local_ip = 0x0A000001U; + rs->if_idx = 0; + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A0000FEU); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), -WOLFIP_EAGAIN); + + /* Same socket, packet destined to the bound IP: delivered. */ + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), delivered); + + /* Catch-all destination but bound to a different interface: filtered. */ + rs->bound_local_ip = IPADDR_ANY; + rs->if_idx = (uint8_t)(TEST_PRIMARY_IF + 1U); + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), -WOLFIP_EAGAIN); + + /* if_idx == 0 means "any interface": delivered on the arriving one. */ + rs->if_idx = 0; + raw_bind_build_frame(&s, frame, sizeof(frame_buf), 0x0A000001U); + wolfIP_recv_ex(&s, TEST_PRIMARY_IF, frame, RAW_BIND_FRAME_LEN); + ck_assert_int_eq(wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + NULL, 0), delivered); +} +END_TEST + START_TEST(test_raw_socket_send_hdrincl_respected) { struct wolfIP s; @@ -6955,6 +7055,102 @@ START_TEST(test_packet_socket_send_frame) } END_TEST +#if WOLFIP_PACKET_SOCKETS +/* F-4501: a SENDING-filter block must not desync the TX walk from fifo_pop(). + * Frame A (PKT_A_LEN) is blocked; frame B (PKT_B_LEN) is accepted. The filter + * counts how many times each length reaches the SENDING hook: the bug walked + * with fifo_next() but removed the tail with fifo_pop(), re-peeking and + * re-sending B (B seen twice). The fix drops the blocked frame and sends B + * exactly once. */ +#define PKT_F4501_A_LEN ((uint32_t)(ETH_HEADER_LEN + 8)) +#define PKT_F4501_B_LEN ((uint32_t)(ETH_HEADER_LEN + 16)) +static int pkt_f4501_block_a_calls; +static int pkt_f4501_b_send_calls; +static int pkt_f4501_filter_cb(void *arg, const struct wolfIP_filter_event *event) +{ + (void)arg; + if (event && event->reason == WOLFIP_FILT_SENDING) { + if (event->length == PKT_F4501_A_LEN) { + pkt_f4501_block_a_calls++; + return 1; /* block frame A */ + } + if (event->length == PKT_F4501_B_LEN) + pkt_f4501_b_send_calls++; + } + return 0; +} + +START_TEST(test_packet_socket_tx_filter_block_does_not_resend) +{ + struct wolfIP s; + int sd; + struct wolfIP_sockaddr_ll sll; + struct wolfIP_sockaddr_ll bind_sll; + uint8_t frame_a[PKT_F4501_A_LEN]; + uint8_t frame_b[PKT_F4501_B_LEN]; + struct wolfIP_eth_frame *eth_a = (struct wolfIP_eth_frame *)frame_a; + struct wolfIP_eth_frame *eth_b = (struct wolfIP_eth_frame *)frame_b; + struct packetsocket *ps; + + wolfIP_init(&s); + mock_link_init(&s); + + sd = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, ee16(ETH_TYPE_IP)); + ck_assert_int_ge(sd, 0); + + memset(&bind_sll, 0, sizeof(bind_sll)); + bind_sll.sll_family = AF_PACKET; + bind_sll.sll_protocol = ee16(ETH_TYPE_IP); + bind_sll.sll_ifindex = TEST_PRIMARY_IF; + bind_sll.sll_halen = 6; + memset(bind_sll.sll_addr, 0xFF, 6); + ck_assert_int_eq(wolfIP_sock_bind(&s, sd, + (struct wolfIP_sockaddr *)&bind_sll, sizeof(bind_sll)), 0); + + memset(&sll, 0, sizeof(sll)); + sll.sll_family = AF_PACKET; + sll.sll_protocol = ee16(ETH_TYPE_IP); + sll.sll_ifindex = TEST_PRIMARY_IF; + sll.sll_halen = 6; + memset(sll.sll_addr, 0xFF, 6); + + /* Queue [A, B]; A is shorter so the filter can tell them apart. */ + memset(frame_a, 0, sizeof(frame_a)); + memcpy(eth_a->dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(eth_a->src, "\x00\x00\x00\x00\x00\x01", 6); + eth_a->type = ee16(ETH_TYPE_IP); + memset(eth_a->data, 0xAA, sizeof(frame_a) - ETH_HEADER_LEN); + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, frame_a, sizeof(frame_a), 0, + (struct wolfIP_sockaddr *)&sll, sizeof(sll)), (int)sizeof(frame_a)); + + memset(frame_b, 0, sizeof(frame_b)); + memcpy(eth_b->dst, "\xff\xff\xff\xff\xff\xff", 6); + memcpy(eth_b->src, "\x00\x00\x00\x00\x00\x02", 6); + eth_b->type = ee16(ETH_TYPE_IP); + memset(eth_b->data, 0xBB, sizeof(frame_b) - ETH_HEADER_LEN); + ck_assert_int_eq(wolfIP_sock_sendto(&s, sd, frame_b, sizeof(frame_b), 0, + (struct wolfIP_sockaddr *)&sll, sizeof(sll)), (int)sizeof(frame_b)); + + pkt_f4501_block_a_calls = 0; + pkt_f4501_b_send_calls = 0; + wolfIP_filter_set_callback(pkt_f4501_filter_cb, NULL); + wolfIP_filter_set_mask(~0U); + + wolfIP_poll(&s, 1000); + + wolfIP_filter_set_callback(NULL, NULL); + wolfIP_filter_set_mask(0); + + /* A was filtered (and dropped), B was sent exactly once - not twice. */ + ck_assert_int_ge(pkt_f4501_block_a_calls, 1); + ck_assert_int_eq(pkt_f4501_b_send_calls, 1); + /* Both descriptors are gone: the blocked one is dropped, not left behind. */ + ps = &s.packetsockets[SOCKET_UNMARK(sd)]; + ck_assert_ptr_eq(fifo_peek(&ps->txbuf), NULL); +} +END_TEST +#endif /* WOLFIP_PACKET_SOCKETS */ + START_TEST(test_packet_socket_sendto_wrong_family_returns_einval) { #if WOLFIP_PACKET_SOCKETS diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 954dc1d0..c70a77ab 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -726,10 +726,14 @@ START_TEST(test_dhcp_parse_ack_ignores_short_unknown_option) opt->data[2] = (offer_ip >> 8) & 0xFF; opt->data[3] = (offer_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0; opt->data[1] = 0; opt->data[2] = 0; opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; - ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 26), 0); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 32), 0); ck_assert_uint_eq(primary->ip, offer_ip); ck_assert_uint_eq(primary->mask, mask); ck_assert_uint_eq(s.dhcp_server_ip, server_ip); @@ -782,10 +786,14 @@ START_TEST(test_dhcp_parse_ack_ignores_zero_len_unknown_option) opt->data[2] = (offer_ip >> 8) & 0xFF; opt->data[3] = (offer_ip >> 0) & 0xFF; opt = (struct dhcp_option *)((uint8_t *)opt + 6); + opt->code = DHCP_OPTION_LEASE_TIME; /* mandatory in a DHCPACK */ + opt->len = 4; + opt->data[0] = 0; opt->data[1] = 0; opt->data[2] = 0; opt->data[3] = 120; + opt = (struct dhcp_option *)((uint8_t *)opt + 6); opt->code = DHCP_OPTION_END; opt->len = 0; - ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 25), 0); + ck_assert_int_eq(dhcp_parse_ack(&s, &msg, DHCP_HEADER_LEN + 31), 0); ck_assert_uint_eq(primary->ip, offer_ip); ck_assert_uint_eq(primary->mask, mask); ck_assert_uint_eq(s.dhcp_server_ip, server_ip); @@ -1410,7 +1418,9 @@ START_TEST(test_ip_recv_forward_ttl_exceeded) ip->ttl = 1; ip->proto = WI_IPPROTO_UDP; ip->len = ee16(IP_HEADER_LEN + 8); - ip->src = ee32(primary_ip); + /* Legitimate in-subnet host source (not the router's own IP, which is + * now dropped as a spoof on the forward path - see F-5697). */ + ip->src = ee32(0x0A000002U); ip->dst = ee32(0xC0A80155U); fix_ip_checksum(ip); @@ -1586,7 +1596,9 @@ START_TEST(test_ip_recv_forward_arp_queue_and_flush) ip.ttl = 2; ip.proto = WI_IPPROTO_TCP; ip.len = ee16(IP_HEADER_LEN); - ip.src = ee32(primary_ip); + /* Legitimate in-subnet host source (not the router's own IP, which is + * now dropped as a spoof on the forward path - see F-5697). */ + ip.src = ee32(0x0A000002U); ip.dst = ee32(dest_ip); fix_ip_checksum(&ip); @@ -4331,6 +4343,48 @@ START_TEST(test_tcp_rebuild_rx_sack_right_edge_wraps) } END_TEST +/* F-5481: RFC 2018 sec.4 (2) - the first SACK block MUST describe the segment + * that triggered the ACK. Store three disjoint islands so descending-sequence + * order would put the most recently received (lowest) island last; the rebuild + * must instead promote the triggering segment's block to rx_sack[0]. */ +START_TEST(test_tcp_rebuild_rx_sack_triggering_block_first) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t payload[16] = {0}; + + 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; + + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 300, 10), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 400, 10), 0); + /* This is the freshest segment and the lowest sequence: under a plain + * descending sort it would be reported last and dropped first on + * option-space truncation. */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 200, 10), 0); + + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 3); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, 200); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].right, 210); + /* Remaining islands keep descending order behind the required first block. */ + ck_assert_uint_eq(ts->sock.tcp.rx_sack[1].left, 400); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[2].left, 300); + + /* Closing holes advances the cumulative ACK (the RFC exemption), so the + * consume path reports remaining islands in plain descending order. */ + ts->sock.tcp.ack = 200; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, 200); + tcp_consume_ooo(ts); + ck_assert_uint_eq(ts->sock.tcp.rx_sack_count, 2); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[0].left, 400); + ck_assert_uint_eq(ts->sock.tcp.rx_sack[1].left, 300); +} +END_TEST + START_TEST(test_tcp_consume_ooo_wrap_trim_and_promote) { struct wolfIP s; @@ -4387,6 +4441,44 @@ START_TEST(test_tcp_consume_ooo_wrap_drop_fully_acked) } END_TEST +/* F-4949: overlapping out-of-order segments with distinct (seq,len) must not + * each consume a slot, or an attacker (or a peer re-segmenting retransmissions) + * could exhaust the OOO cache and get all later legitimate OOO data dropped. */ +START_TEST(test_tcp_store_ooo_overlap_does_not_exhaust_cache) +{ + struct wolfIP s; + struct tsocket *ts; + uint8_t payload[100] = {0}; + int i, used; + + 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 = 1000; + + /* Four overlapping segments covering essentially the same bytes. */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1100, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1101, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1200, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 1201, 100), 0); + + used = 0; + for (i = 0; i < TCP_OOO_MAX_SEGS; i++) + if (ts->sock.tcp.ooo[i].used) + used++; + /* The overlapping cluster must leave free slots for real OOO data. */ + ck_assert_int_lt(used, TCP_OOO_MAX_SEGS); + + /* Legitimate non-overlapping OOO segments are still accepted (not dropped + * by a cache exhausted with overlapping junk). */ + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 2000, 100), 0); + ck_assert_int_eq(tcp_store_ooo_segment(ts, payload, 2200, 100), 0); +} +END_TEST + START_TEST(test_tcp_ack_sack_early_retransmit_before_three_dupack) { struct wolfIP s; diff --git a/src/test/unit/unit_tests_tcp_state.c b/src/test/unit/unit_tests_tcp_state.c index 7be2fc2e..bf273b47 100644 --- a/src/test/unit/unit_tests_tcp_state.c +++ b/src/test/unit/unit_tests_tcp_state.c @@ -396,6 +396,45 @@ START_TEST(test_tcp_parse_options_timestamp_parsed) } END_TEST +/* Overlong Timestamp option (olen > canonical 10) must be rejected. + * RFC 7323 fixes the TS option at length 10; only canonical lengths + * should roundtrip. An olen=11 TS must not negotiate ts_enabled. */ +START_TEST(test_tcp_parse_options_timestamp_overlong_ignored) +{ + /* TS option with olen=11 (one byte longer than canonical). The 9 + * trailing bytes still carry a well-formed TSval/TSEcr; only the + * length is wrong. */ + uint8_t opts[] = { + TCP_OPTION_TS, 11, + 0x00, 0x00, 0x01, 0x02, /* TSval = 0x0102 */ + 0x00, 0x00, 0x00, 0x00, /* TSEcr = 0 */ + 0x00, /* extra byte: olen=11, not 10 */ + TCP_OPTION_EOO + }; + struct wolfIP s; + struct tsocket *ts; + + 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_LISTEN; + ts->src_port = 8080; + ts->sock.tcp.sack_offer = 1; + + inject_tcp_segment_with_opts(&s, TEST_PRIMARY_IF, + 0x0A0000A1U, 0x0A000001U, 40000, 8080, + 1, 0, TCP_FLAG_SYN, + opts, (uint8_t)sizeof(opts), NULL, 0); + + ck_assert_int_eq(ts->sock.tcp.ts_enabled, 0); +} +END_TEST + /* MSS option value 0 is ignored (falls back to default) */ START_TEST(test_tcp_parse_options_mss_zero_ignored) { diff --git a/src/test/unit/unit_tests_tftp.c b/src/test/unit/unit_tests_tftp.c index e1207cd6..fb860528 100644 --- a/src/test/unit/unit_tests_tftp.c +++ b/src/test/unit/unit_tests_tftp.c @@ -330,6 +330,62 @@ START_TEST(test_tftp_parse_request_error_paths) } END_TEST +/* F-5009: an unauthenticated RRQ/WRQ filename must not be able to escape the + * integrator's namespace. parse_request rejects any '..' path component and any + * absolute path before the name can reach io.open. Benign names that merely + * contain dots (but no '..' component) and relative subdirectories still pass. */ +START_TEST(test_tftp_parse_request_rejects_path_traversal) +{ + uint8_t pkt[160]; + struct wolftftp_parsed_req req; + static const char *bad[] = { + "../../etc/passwd", /* leading traversal */ + "/etc/cron.d/evil", /* absolute (unix) */ + "\\windows\\sys", /* absolute (dos) */ + "..", /* whole name is a traversal */ + "fw/../../secret", /* embedded traversal */ + "images/..", /* trailing traversal component */ + "a/..\\b", /* backslash-separated traversal*/ + "C:\\windows\\sys", /* windows drive-letter absolute*/ + "C:fw.bin", /* windows drive-relative path */ + "fw.bin:stream", /* ntfs alternate data stream */ + }; + static const char *good[] = { + "fw.bin", /* ordinary name */ + "images/fw.bin", /* relative subdir is fine */ + "fw..bin", /* dots, but not a '..' component */ + "v1.2..3.img", /* same */ + ".config", /* leading dot, not traversal */ + }; + size_t i, fnlen; + + for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) { + memset(pkt, 0, sizeof(pkt)); + wolftftp_write_u16(pkt, WOLFTFTP_OP_RRQ); + fnlen = strlen(bad[i]); + memcpy(pkt + 2, bad[i], fnlen + 1U); + memcpy(pkt + 2 + fnlen + 1U, "octet", 6); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), WOLFTFTP_ERR_PACKET); + /* Same rejection on the WRQ (arbitrary-write) path. */ + wolftftp_write_u16(pkt, WOLFTFTP_OP_WRQ); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), WOLFTFTP_ERR_PACKET); + } + + for (i = 0; i < sizeof(good) / sizeof(good[0]); i++) { + memset(pkt, 0, sizeof(pkt)); + wolftftp_write_u16(pkt, WOLFTFTP_OP_RRQ); + fnlen = strlen(good[i]); + memcpy(pkt + 2, good[i], fnlen + 1U); + memcpy(pkt + 2 + fnlen + 1U, "octet", 6); + ck_assert_int_eq(wolftftp_parse_request(pkt, + (uint16_t)(2U + fnlen + 1U + 6U), &req), 0); + ck_assert_str_eq(req.filename, good[i]); + } +} +END_TEST + START_TEST(test_tftp_client_rrq_oack_and_data_success) { struct tftp_test_ctx ctx; @@ -2045,6 +2101,7 @@ static void add_tftp_tests(TCase *tc_proto) { tcase_add_test(tc_proto, test_tftp_helpers_and_builders); tcase_add_test(tc_proto, test_tftp_parse_request_error_paths); + tcase_add_test(tc_proto, test_tftp_parse_request_rejects_path_traversal); tcase_add_test(tc_proto, test_tftp_client_rrq_oack_and_data_success); tcase_add_test(tc_proto, test_tftp_client_fallback_duplicate_and_tid_errors); tcase_add_test(tc_proto, test_tftp_client_error_and_failure_paths); diff --git a/src/test/unit/unit_tests_vlan.c b/src/test/unit/unit_tests_vlan.c index 1bed48b4..382807ed 100644 --- a/src/test/unit/unit_tests_vlan.c +++ b/src/test/unit/unit_tests_vlan.c @@ -948,6 +948,125 @@ START_TEST(test_vlan_rx_untagged_on_physical_ok) } END_TEST +#if WOLFIP_PACKET_SOCKETS +/* F-5011: a tagged frame is rebased onto the sub-interface before + * packet_try_recv ran, so an AF_PACKET socket bound to the parent saw nothing. + * After the fix the parent (and wildcard) listeners observe the original tagged + * frame, while a socket bound to the sub-interface still gets the tag-stripped + * copy. */ +START_TEST(test_vlan_packet_socket_parent_and_sub_delivery) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + int sd_parent, sd_sub; + uint8_t plain[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN]; + uint8_t tagged[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN + 4]; + uint32_t plain_len, tagged_len; + uint8_t rxbuf[sizeof(tagged) + 4]; + struct wolfIP_sockaddr_ll from; + socklen_t from_len; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + + /* protocol 0 == ETH_P_ALL so both the 0x8100 (tagged) and 0x0800 + * (stripped) views match. */ + sd_parent = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd_parent, 0); + s.packetsockets[SOCKET_UNMARK(sd_parent)].bind_addr.sll_ifindex = + (int)TEST_PRIMARY_IF; + sd_sub = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd_sub, 0); + s.packetsockets[SOCKET_UNMARK(sd_sub)].bind_addr.sll_ifindex = (int)sub_idx; + + plain_len = build_icmp_echo_request(plain, sizeof(plain), phys->mac, + VLAN_REMOTE_IP, VLAN_SUB100_IP); + ck_assert_uint_gt(plain_len, 0u); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, plain_len, + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + + /* Parent-bound socket: original tagged frame, stamped with the parent. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd_parent, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)tagged_len); + ck_assert_uint_eq(rxbuf[12], 0x81u); /* VLAN TPID still present */ + ck_assert_uint_eq(rxbuf[13], 0x00u); + ck_assert_int_eq(from.sll_ifindex, (int)TEST_PRIMARY_IF); + + /* Sub-interface-bound socket: tag-stripped frame, stamped with the sub. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd_sub, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)(tagged_len - 4)); + ck_assert_uint_eq(rxbuf[12], 0x08u); /* inner IPv4 ethertype, tag gone */ + ck_assert_uint_eq(rxbuf[13], 0x00u); + ck_assert_int_eq(from.sll_ifindex, (int)sub_idx); +} +END_TEST + +/* F-5011: a wildcard (sll_ifindex == 0) sniffer must receive the original + * tagged frame exactly once - stamped with the parent - not the tag-stripped + * sub-interface copy, and not both. */ +START_TEST(test_vlan_packet_socket_wildcard_gets_tagged_once) +{ + struct wolfIP s; + struct wolfIP_ll_dev *phys; + unsigned int sub_idx = 0xFFFFFFFFu; + int sd; + uint8_t plain[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN]; + uint8_t tagged[ETH_HEADER_LEN + IP_HEADER_LEN + ICMP_HEADER_LEN + 4]; + uint32_t plain_len, tagged_len; + uint8_t rxbuf[sizeof(tagged) + 4]; + struct wolfIP_sockaddr_ll from; + socklen_t from_len; + int ret; + + setup_vlan_stack(&s); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub_idx); + ck_assert_int_eq(ret, 0); + phys = wolfIP_getdev_ex(&s, TEST_PRIMARY_IF); + ck_assert_ptr_nonnull(phys); + + sd = wolfIP_sock_socket(&s, AF_PACKET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(sd, 0); + s.packetsockets[SOCKET_UNMARK(sd)].bind_addr.sll_ifindex = 0; /* wildcard */ + + plain_len = build_icmp_echo_request(plain, sizeof(plain), phys->mac, + VLAN_REMOTE_IP, VLAN_SUB100_IP); + ck_assert_uint_gt(plain_len, 0u); + tagged_len = insert_vlan_tag(tagged, sizeof(tagged), plain, plain_len, + 100, 0, 0); + ck_assert_uint_gt(tagged_len, 0u); + + wolfIP_recv_on(&s, TEST_PRIMARY_IF, tagged, tagged_len); + + /* First (and only) delivery: the tagged wire frame on the parent. */ + memset(&from, 0, sizeof(from)); + from_len = sizeof(from); + ret = wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, + (struct wolfIP_sockaddr *)&from, &from_len); + ck_assert_int_eq(ret, (int)tagged_len); + ck_assert_uint_eq(rxbuf[12], 0x81u); + ck_assert_int_eq(from.sll_ifindex, (int)TEST_PRIMARY_IF); + + /* No duplicate tag-stripped copy. */ + ret = wolfIP_sock_recvfrom(&s, sd, rxbuf, sizeof(rxbuf), 0, NULL, 0); + ck_assert_int_eq(ret, -WOLFIP_EAGAIN); +} +END_TEST +#endif /* WOLFIP_PACKET_SOCKETS */ + START_TEST(test_vlan_rx_runt_tagged_dropped) { struct wolfIP s; @@ -1187,4 +1306,39 @@ START_TEST(test_vlan_mtu_inherited) } END_TEST +/* F-4948: deleting a VLAN sub-interface must purge the ARP neighbor cache for + * that slot. ARP entries are keyed only by (ip, if_idx) with no VID, and + * wolfIP_vlan_create reuses the freed slot, so without a purge a new VLAN on + * the same if_idx would inherit the deleted VLAN's L2 mappings. */ +START_TEST(test_vlan_delete_purges_arp_neighbor_cache) +{ + struct wolfIP s; + unsigned int sub100 = 0xFFFFFFFFu, sub200 = 0xFFFFFFFFu; + static const uint8_t peer_mac[6] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF}; + const ip4 peer_ip = VLAN_REMOTE_IP; + uint8_t mac[6]; + int ret; + + setup_vlan_stack(&s); + + /* Learn a neighbor on a VID=100 sub-interface. */ + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 100, 0, 0, &sub100); + ck_assert_int_eq(ret, 0); + arp_store_neighbor(&s, sub100, peer_ip, peer_mac); + ck_assert_int_eq(wolfIP_arp_lookup_ex(&s, sub100, peer_ip, mac), 0); + ck_assert_mem_eq(mac, peer_mac, 6); + + /* Delete VID=100 and recreate as VID=200 on the same slot. */ + ck_assert_int_eq(wolfIP_vlan_delete(&s, sub100), 0); + ret = wolfIP_vlan_create(&s, TEST_PRIMARY_IF, 200, 0, 0, &sub200); + ck_assert_int_eq(ret, 0); + ck_assert_uint_eq(sub200, sub100); /* slot reused */ + + /* The new VLAN must not inherit the deleted VLAN's L2 mapping. */ + memset(mac, 0, sizeof(mac)); + ret = wolfIP_arp_lookup_ex(&s, sub200, peer_ip, mac); + ck_assert_int_eq(ret, -1); +} +END_TEST + #endif /* WOLFIP_VLAN */ diff --git a/src/tftp/wolftftp.c b/src/tftp/wolftftp.c index ee78caeb..a43723c8 100644 --- a/src/tftp/wolftftp.c +++ b/src/tftp/wolftftp.c @@ -272,6 +272,43 @@ static int wolftftp_build_request(uint8_t *buf, uint16_t max_len, uint16_t opcod return 0; } +/* Reject filenames that could escape the integrator's namespace: any absolute + * path (leading '/' or '\', or a Windows drive specifier like "C:\...") and any + * ".." path component. The library hands the name straight to io.open(), and + * TFTP is unauthenticated, so a naive filesystem-backed open() would otherwise + * be exposed to traversal. The check is component-aware: a ".." segment between + * separators (or as the whole name) is rejected, but dots inside a name + * ("fw..bin") are fine. */ +static int wolftftp_filename_is_safe(const char *name) +{ + const char *seg = name; + const char *c; + + if (name[0] == '\0') + return 0; + if (name[0] == '/' || name[0] == '\\') + return 0; + /* ':' is never valid in a portable filename; on Windows it introduces a + * drive specifier ("C:\windows", "C:foo") or an NTFS alternate data stream + * ("name:stream"), either of which would sidestep the leading-separator + * check above. Reject it wherever it appears. */ + for (c = name; *c != '\0'; c++) { + if (*c == ':') + return 0; + } + for (c = name; ; c++) { + if (*c == '/' || *c == '\\' || *c == '\0') { + size_t seglen = (size_t)(c - seg); + if (seglen == 2U && seg[0] == '.' && seg[1] == '.') + return 0; + if (*c == '\0') + break; + seg = c + 1; + } + } + return 1; +} + static int wolftftp_parse_request(const uint8_t *buf, uint16_t len, struct wolftftp_parsed_req *req) { @@ -294,6 +331,8 @@ static int wolftftp_parse_request(const uint8_t *buf, uint16_t len, return WOLFTFTP_ERR_PACKET; memcpy(req->filename, p, slen); req->filename[slen] = '\0'; + if (!wolftftp_filename_is_safe(req->filename)) + return WOLFTFTP_ERR_PACKET; p += slen + 1U; slen = wolftftp_strnlen_local(p, (size_t)(buf + len - (const uint8_t *)p)); if (slen == 0 || wolftftp_stricmp_local(p, "octet") != 0) diff --git a/src/tftp/wolftftp.h b/src/tftp/wolftftp.h index eba2e4d5..085efea7 100644 --- a/src/tftp/wolftftp.h +++ b/src/tftp/wolftftp.h @@ -143,6 +143,12 @@ struct wolftftp_negotiated { typedef int (*wolftftp_udp_send_cb)(void *arg, uint16_t local_port, const struct wolftftp_endpoint *remote, const uint8_t *buf, uint16_t len); +/* wolftftp_open_cb: + * `name` is the wire-supplied request filename. The server rejects absolute + * paths and any ".." path component before this callback runs, so `name` + * cannot escape upward from a relative root. It may still contain relative + * subdirectories ("a/b/fw.bin"); a filesystem-backed open() should resolve it + * against a confined root (chroot / fixed base dir) rather than the cwd. */ typedef int (*wolftftp_open_cb)(void *arg, const char *name, int is_write, uint32_t *size_hint, void **handle); /* wolftftp_read_cb: diff --git a/src/wolfip.c b/src/wolfip.c index 9d303ff2..d64846d9 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2604,13 +2604,20 @@ static void raw_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip if (ip_len > payload_len) return; payload_len = ip_len; - (void)if_idx; for (int i = 0; i < WOLFIP_MAX_RAWSOCKETS; i++) { struct rawsocket *r = &s->rawsockets[i]; if (!r->used) continue; if (r->protocol != 0 && r->protocol != ip->proto) continue; + /* Honour the bind contract, mirroring the TCP/UDP receive paths: a + * socket bound to a specific local IP or interface must not capture + * traffic for other destinations or arriving on other interfaces. + * bound_local_ip == IPADDR_ANY / if_idx == 0 mean "any". */ + if (r->bound_local_ip != IPADDR_ANY && r->bound_local_ip != ee32(ip->dst)) + continue; + if (r->if_idx != 0 && r->if_idx != (uint8_t)if_idx) + continue; if (fifo_push(&r->rxbuf, (void *)packet, payload_len) == 0) { r->last_pkt_ttl = ip->ttl; r->events |= CB_EVENT_READABLE; @@ -2620,7 +2627,7 @@ static void raw_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip #endif #if WOLFIP_PACKET_SOCKETS -static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_eth_frame *eth, uint32_t frame_len) +static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP_eth_frame *eth, uint32_t frame_len, int match_wildcard) { uint16_t proto = eth->type; uint32_t record_len; @@ -2639,10 +2646,22 @@ static void packet_try_recv(struct wolfIP *s, unsigned int if_idx, struct wolfIP continue; if (p->protocol && p->protocol != proto) continue; - if ((p->bind_addr.sll_ifindex >= 0) && - (unsigned int)p->bind_addr.sll_ifindex != if_idx && - p->bind_addr.sll_ifindex != 0) - continue; + if (match_wildcard) { + /* Deliver to sockets bound to this interface, plus wildcard + * (sll_ifindex == 0) and unbound (sll_ifindex < 0) listeners. */ + if ((p->bind_addr.sll_ifindex >= 0) && + (unsigned int)p->bind_addr.sll_ifindex != if_idx && + p->bind_addr.sll_ifindex != 0) + continue; + } else { + /* Deliver only to sockets bound explicitly to this interface. + * Used for the post-VLAN-demux pass so wildcard sockets, which + * already saw the original tagged frame on the parent, are not + * delivered the tag-stripped copy a second time. */ + if (p->bind_addr.sll_ifindex < 0 || + (unsigned int)p->bind_addr.sll_ifindex != if_idx) + continue; + } if (fifo_space(&p->rxbuf) < record_len + sizeof(struct pkt_desc)) continue; if (fifo_push(&p->rxbuf, record, record_len) == 0) @@ -2859,7 +2878,7 @@ static void tcp_parse_options(const struct wolfIP_tcp_seg *tcp, uint32_t frame_l po->sack_count++; } } - } else if (kind == TCP_OPTION_TS && olen >= TCP_OPTION_TS_LEN) { + } else if (kind == TCP_OPTION_TS && olen == TCP_OPTION_TS_LEN) { uint32_t val, ecr; memcpy(&val, opt + 2, sizeof(val)); memcpy(&ecr, opt + 6, sizeof(ecr)); @@ -2910,7 +2929,8 @@ static uint8_t tcp_merge_sack_blocks(struct tcp_sack_block *blocks, uint8_t coun return (uint8_t)(out + 1); } -static void tcp_rebuild_rx_sack(struct tsocket *t) +static void tcp_rebuild_rx_sack(struct tsocket *t, uint32_t trig_seq, + uint32_t trig_len) { struct tcp_sack_block blocks[TCP_OOO_MAX_SEGS]; uint8_t i, count = 0; @@ -2936,6 +2956,26 @@ static void tcp_rebuild_rx_sack(struct tsocket *t) count--; t->sock.tcp.rx_sack[t->sock.tcp.rx_sack_count++] = blocks[count]; } + + /* RFC 2018 sec.4 (2): the first SACK block MUST contain the segment that + * triggered this ACK (unless that segment advanced the cumulative ACK, in + * which case the caller passes trig_len == 0). The loop above leaves blocks + * in descending-sequence order, so the triggering island can be anywhere; + * move the merged block holding it to the front. This also guarantees it + * survives truncation when the TCP option lacks room for every block. */ + if (trig_len != 0U) { + for (i = 0; i < t->sock.tcp.rx_sack_count; i++) { + struct tcp_sack_block *b = &t->sock.tcp.rx_sack[i]; + if (tcp_seq_leq(b->left, trig_seq) && tcp_seq_lt(trig_seq, b->right)) { + struct tcp_sack_block first = t->sock.tcp.rx_sack[i]; + uint8_t j; + for (j = i; j > 0; j--) + t->sock.tcp.rx_sack[j] = t->sock.tcp.rx_sack[j - 1]; + t->sock.tcp.rx_sack[0] = first; + break; + } + } + } } static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, @@ -2961,11 +3001,36 @@ static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, slot = (int)i; continue; } - /* Duplicate range: keep newest bytes and avoid consuming another slot. */ - if (t->sock.tcp.ooo[i].seq == seq && t->sock.tcp.ooo[i].len == len) { - memcpy(t->sock.tcp.ooo[i].data, data, len); - tcp_rebuild_rx_sack(t); - return 0; + /* Overlapping range (including an exact duplicate): coalesce into this + * slot rather than consuming another. Otherwise an attacker injecting + * distinct (seq,len) pairs over the same bytes - or a peer that + * re-segments retransmissions - could occupy every slot with overlapping + * data and starve later legitimate OOO segments. The union of the two + * ranges is stored in place (incoming bytes win in the overlap) when it + * fits one slot; if it would not fit, fall through and keep them as + * separate slots (tcp_consume_ooo coalesces them on promotion). */ + { + uint32_t cur_seq = t->sock.tcp.ooo[i].seq; + uint32_t cur_len = t->sock.tcp.ooo[i].len; + uint32_t end_new = tcp_seq_inc(seq, len); + uint32_t end_cur = tcp_seq_inc(cur_seq, cur_len); + if (tcp_seq_lt(seq, end_cur) && tcp_seq_lt(cur_seq, end_new)) { + uint32_t u_start = tcp_seq_lt(seq, cur_seq) ? seq : cur_seq; + uint32_t u_end = tcp_seq_lt(end_cur, end_new) ? end_new : end_cur; + uint32_t u_len = (uint32_t)tcp_seq_diff(u_end, u_start); + if (u_len <= TCP_MSS_MAX) { + uint8_t *buf = t->sock.tcp.ooo[i].data; + uint32_t cur_off = (uint32_t)tcp_seq_diff(cur_seq, u_start); + uint32_t new_off = (uint32_t)tcp_seq_diff(seq, u_start); + if (cur_off != 0) + memmove(buf + cur_off, buf, cur_len); + memcpy(buf + new_off, data, len); + t->sock.tcp.ooo[i].seq = u_start; + t->sock.tcp.ooo[i].len = u_len; + tcp_rebuild_rx_sack(t, u_start, u_len); + return 0; + } + } } } if (slot < 0) @@ -2975,7 +3040,7 @@ static int tcp_store_ooo_segment(struct tsocket *t, const uint8_t *data, t->sock.tcp.ooo[slot].seq = seq; t->sock.tcp.ooo[slot].len = len; memcpy(t->sock.tcp.ooo[slot].data, data, len); - tcp_rebuild_rx_sack(t); + tcp_rebuild_rx_sack(t, seq, len); return 0; } @@ -3039,8 +3104,10 @@ static void tcp_consume_ooo(struct tsocket *t) } } /* Rebuild advertised SACK blocks from whatever OOO cache remains after - * promotion. If all holes closed, this naturally drops SACK reporting. */ - tcp_rebuild_rx_sack(t); + * promotion. If all holes closed, this naturally drops SACK reporting. + * Promotion advances the cumulative ACK, so RFC 2018's "first block holds + * the triggering segment" rule does not apply here (trig_len == 0). */ + tcp_rebuild_rx_sack(t, 0, 0); } static uint8_t tcp_build_ack_options(struct tsocket *t, uint8_t *opt, uint8_t max_len) @@ -3817,6 +3884,11 @@ static void iphdr_set_checksum(struct wolfIP_ip_packet *ip) if (ip_hlen < IP_HEADER_LEN) ip_hlen = IP_HEADER_LEN; + /* Zero the checksum field before summing (RFC 1071) so the result is + * correct regardless of any stale value the caller left in ip->csum. This + * makes the setter idempotent (set/verify always holds) and removes the + * implicit "caller must clear csum first" precondition. */ + ip->csum = 0; for (i = 0; i < ip_hlen; i += 2) { uint16_t word; memcpy(&word, ptr + i, sizeof(word)); @@ -3970,6 +4042,12 @@ static void igmp_input(struct wolfIP *s, unsigned int if_idx, return; if (igmp[0] != IGMP_TYPE_MEMBERSHIP_QUERY) return; + /* RFC 3376 §4.1.1 / RFC 2236 §2: IGMP messages carry IP TTL 1 and are + * link-local; a query with any other TTL transited a router and cannot be + * a legitimate on-link query. Dropping it stops an off-link/spoofed query + * from soliciting (and thereby disclosing) the host's membership reports. */ + if (ip->ttl != 1) + return; /* RFC 2236 §2 (IGMPv2) and RFC 3376 §4.1 (IGMPv3) both place the Group * Address at offset 4 within the message. Read unconditionally so that * IGMPv1/v2 group-specific queries (8-byte messages) are not silently @@ -3977,6 +4055,14 @@ static void igmp_input(struct wolfIP *s, unsigned int if_idx, group = get_be32(igmp + 4); if (group != IPADDR_ANY && !wolfIP_ip_is_multicast(group)) return; + /* RFC 3376 §4.1.2: a general query is addressed to all-hosts (224.0.0.1) + * and a group-specific query to the group itself. Reject a query sent to + * any other destination (e.g. our unicast address). */ + { + ip4 dst = ee32(ip->dst); + if (dst != IGMP_ALL_HOSTS && dst != group) + return; + } for (i = 0; i < WOLFIP_MCAST_MEMBERSHIPS; i++) { if (s->mcast[i].refs == 0 || s->mcast[i].if_idx != if_idx) @@ -5560,29 +5646,39 @@ int wolfIP_sock_connect(struct wolfIP *s, int sockfd, const struct wolfIP_sockad return -WOLFIP_EINVAL; if (ts->sock.tcp.state == TCP_CLOSED) { struct ipconf *conf; - ts->sock.tcp.state = TCP_SYN_SENT; - ts->remote_ip = ee32(sin->sin_addr.s_addr); + ip4 new_remote_ip = ee32(sin->sin_addr.s_addr); + uint8_t new_if_idx; + ip4 new_local_ip; + + /* Resolve and validate the local binding into locals before mutating + * the socket. A failed validation here must not leave the socket in + * TCP_SYN_SENT (no SYN queued, no RTO timer), which would make every + * later connect return EAGAIN forever. Mirrors the UDP arm above. */ if (ts->bound_local_ip != IPADDR_ANY) { int bound_match = 0; unsigned int bound_if = wolfIP_if_for_local_ip(s, ts->bound_local_ip, &bound_match); if (!bound_match) return -WOLFIP_EINVAL; - ts->if_idx = (uint8_t)bound_if; - ts->local_ip = ts->bound_local_ip; + new_if_idx = (uint8_t)bound_if; + new_local_ip = ts->bound_local_ip; } else { - if_idx = wolfIP_route_for_ip(s, ts->remote_ip); + if_idx = wolfIP_route_for_ip(s, new_remote_ip); conf = wolfIP_ipconf_at(s, if_idx); - ts->if_idx = (uint8_t)if_idx; + new_if_idx = (uint8_t)if_idx; if (conf && conf->ip != IPADDR_ANY) - ts->local_ip = conf->ip; + new_local_ip = conf->ip; else { struct ipconf *primary = wolfIP_primary_ipconf(s); if (primary && primary->ip != IPADDR_ANY) - ts->local_ip = primary->ip; + new_local_ip = primary->ip; else - ts->local_ip = IPADDR_ANY; + new_local_ip = IPADDR_ANY; } } + ts->sock.tcp.state = TCP_SYN_SENT; + ts->remote_ip = new_remote_ip; + ts->if_idx = new_if_idx; + ts->local_ip = new_local_ip; if (!ts->src_port) ts->src_port = (uint16_t)(wolfIP_getrandom() & 0xFFFF); if (ts->src_port < 1024) @@ -6107,6 +6203,13 @@ int wolfIP_sock_recvfrom(struct wolfIP *s, int sockfd, void *buf, size_t len, in if (!s) return -WOLFIP_EINVAL; + /* A nonzero-length read needs a destination buffer: every socket family + * below copies queued data into buf (queue_pop / memcpy). Reject a NULL + * buffer here so caller misuse cannot dereference it. len == 0 (a probe) + * stays allowed. */ + if (!buf && len > 0) + return -WOLFIP_EINVAL; + if (IS_SOCKET_TCP(sockfd)) { if (SOCKET_UNMARK(sockfd) >= MAX_TCPSOCKETS) return -WOLFIP_EINVAL; @@ -7064,7 +7167,11 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->local_ip = prev_ip; return -1; } - ts->src_port = new_port; + /* Commit src_port only after the filter approves the bind (as the + * TCP arm does): otherwise the socket is matchable by the ingress + * path while the BINDING callback runs, and a callback that + * re-enters wolfIP_poll would get a datagram delivered to it even + * if the bind is ultimately rejected. */ if (wolfIP_filter_notify_socket_event( WOLFIP_FILT_BINDING, s, ts, ts->local_ip, new_port, IPADDR_ANY, 0) != 0) { @@ -7072,6 +7179,7 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->src_port = prev_port; return -1; } + ts->src_port = new_port; } ts->bound_local_ip = bind_ip; return 0; @@ -7102,7 +7210,8 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->local_ip = prev_ip; return -1; } - ts->src_port = new_id; + /* Commit the echo id only after the filter approves (see the UDP + * arm): keep the socket unmatchable by icmp_try_recv until then. */ if (wolfIP_filter_notify_socket_event( WOLFIP_FILT_BINDING, s, ts, ts->local_ip, new_id, IPADDR_ANY, 0) != 0) { @@ -7110,6 +7219,7 @@ int wolfIP_sock_bind(struct wolfIP *s, int sockfd, const struct wolfIP_sockaddr ts->src_port = prev_id; return -1; } + ts->src_port = new_id; } ts->bound_local_ip = bind_ip; return 0; @@ -7227,6 +7337,20 @@ static void icmp_input(struct wolfIP *s, unsigned int if_idx, struct wolfIP_ip_p if (wolfIP_filter_notify_icmp(WOLFIP_FILT_RECEIVING, s, if_idx, icmp, len) != 0) return; if (icmp->type == ICMP_ECHO_REPLY) { + ip4 dst = ee32(ip->dst); + int dst_match = 0; + /* RFC 1122 §3.2.2.6: only accept an echo reply that is actually + * addressed to one of our configured local IPs - the same guard the + * ECHO_REQUEST path below applies. Without it, an L2-adjacent attacker + * can address a frame to our MAC with an arbitrary ip.dst and a guessed + * echo id and have a forged reply delivered to an application ICMP + * socket (icmp_try_recv skips the per-socket dst check when the socket's + * local_ip is 0, e.g. during/after DHCP). */ + if (wolfIP_ip_is_broadcast(s, dst) || wolfIP_ip_is_multicast(dst)) + return; + (void)wolfIP_if_for_local_ip(s, dst, &dst_match); + if (!dst_match) + return; icmp_try_recv(s, if_idx, icmp, len); return; } @@ -7415,6 +7539,12 @@ static void dhcp_timer_cb(void *arg) s->dhcp_state = DHCP_RENEWING; s->dhcp_start_tick = s->last_tick; s->dhcp_timeout_count = 0; + /* RFC 2131: a renewal is a new transaction. Pick a fresh, random + * transaction ID so an attacker who observed the initial (broadcast) + * DORA xid cannot blindly forge a renewal DHCPACK for the lifetime + * of the lease. Retransmissions within this cycle (and the + * RENEWING->REBINDING continuation) keep this xid. */ + s->dhcp_xid = wolfIP_getrandom(); ret = dhcp_send_request(s); if (ret >= 0) s->dhcp_timeout_count++; @@ -7751,7 +7881,12 @@ static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_l } if (!saw_end) return -1; - if (primary && saw_server_id && + /* RFC 2131: the IP-address-lease-time option (51) is mandatory + * in a DHCPACK. lease_s is only ever set by that option (and a + * short option already returns -1 above), so lease_s != 0 means + * it was present with a valid nonzero duration. Without it the + * lease would be bound with no expiry/renewal timer. */ + if (primary && saw_server_id && lease_s != 0 && (primary->ip != 0) && (primary->mask != 0)) { dhcp_cancel_timer(s); s->dhcp_state = DHCP_BOUND; @@ -7982,6 +8117,8 @@ static int dhcp_send_discover(struct wolfIP *s) int dhcp_bound(struct wolfIP *s) { + if (!s) + return 0; return (s->dhcp_state == DHCP_BOUND || s->dhcp_state == DHCP_RENEWING || s->dhcp_state == DHCP_REBINDING); @@ -7989,12 +8126,16 @@ int dhcp_bound(struct wolfIP *s) int dhcp_client_is_running(struct wolfIP *s) { + if (!s) + return 0; return DHCP_IS_RUNNING(s); } int dhcp_client_init(struct wolfIP *s) { struct wolfIP_sockaddr_in sin; + if (!s) + return -WOLFIP_EINVAL; if (s->dhcp_state != DHCP_OFF) return -1; s->dhcp_xid = wolfIP_getrandom(); @@ -8496,6 +8637,34 @@ int wolfIP_vlan_delete(struct wolfIP *s, unsigned int if_idx) * renumbering active sub-ifaces. */ memset(slot, 0, sizeof(*slot)); memset(&s->ipconf[if_idx], 0, sizeof(s->ipconf[if_idx])); + /* Purge ARP state tied to this slot. Neighbor entries are keyed only by + * (ip, if_idx) with no VID, and wolfIP_vlan_create reuses the freed slot, + * so without this a new VLAN on the same if_idx would silently inherit the + * deleted VLAN's L2 mappings (and its queued/in-flight ARP state). */ + { + int i; + for (i = 0; i < MAX_NEIGHBORS; i++) { + if (s->arp.neighbors[i].if_idx == (uint8_t)if_idx) { + s->arp.neighbors[i].ip = IPADDR_ANY; + s->arp.neighbors[i].if_idx = 0; + s->arp.neighbors[i].ts = 0; + memset(s->arp.neighbors[i].mac, 0, 6); + } + } + for (i = 0; i < WOLFIP_ARP_PENDING_MAX; i++) { + if (s->arp.pending[i].if_idx == (uint8_t)if_idx) { + s->arp.pending[i].ip = IPADDR_ANY; + s->arp.pending[i].if_idx = 0; + s->arp.pending[i].ts = 0; + } + if (s->arp_pending[i].if_idx == (uint8_t)if_idx) { + s->arp_pending[i].dest = IPADDR_ANY; + s->arp_pending[i].len = 0; + s->arp_pending[i].if_idx = 0; + } + } + s->arp.last_arp[if_idx] = 0; + } return 0; } @@ -8637,6 +8806,12 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (wolfIP_filter_notify_ip(WOLFIP_FILT_RECEIVING, s, if_idx, ip, len) != 0) return; #if WOLFIP_RAWSOCKETS + /* Raw sockets are a passive ingress tap and intentionally observe traffic + * before the option/forwarding policy below: this is what makes them + * useful for monitoring/IDS (e.g. seeing source-routed attack packets the + * stack itself refuses to act on). raw_try_recv only copies the frame to + * the socket queue; it never parses IP options or honours a source route, + * so tapping ahead of the LSRR/SSRR drop cannot make the stack act on one. */ raw_try_recv(s, if_idx, ip, len); #endif /* RFC 7126 section 3.8: drop source-routed (LSRR/SSRR) packets before either @@ -8694,6 +8869,23 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, if (!rpf_drop && (src & 0xFFFF0000U) == 0xA9FE0000U) { rpf_drop = 1; } + /* Spoofed self: a source equal to one of our own configured + * interface addresses can only be forged - the router originates + * such packets locally, it never receives them from the wire. The + * strict-RPF loop below skips the ingress interface (i == if_idx), + * so its own address would otherwise pass; check every interface's + * own /32 here explicitly. */ + if (!rpf_drop) { + for (i = 0; i < s->if_count; i++) { + struct ipconf *conf = &s->ipconf[i]; + if (!conf || conf->ip == IPADDR_ANY) + continue; + if (conf->ip == src) { + rpf_drop = 1; + break; + } + } + } /* Strict RPF: a source that belongs to some other configured * interface's local subnet must not arrive on this one. */ if (!rpf_drop) { @@ -8747,23 +8939,6 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, wolfIP_print_ip(ip); #endif /* DEBUG_IP*/ - #ifdef WOLFIP_ESP - /* note: esp transport mode only handled here. - * ip forwarding would require esp tunnel mode. */ - if (ip->proto == 0x32) { - int err; - if (wolfIP_ll_is_non_ethernet(s, if_idx)) { - return; - } - /* proto is ESP 0x32 (50), try to unwrap. */ - err = esp_transport_unwrap(ip, &len); - if (err) { - LOG("info: failed to unwrap esp packet, dropping.\n"); - return; - } - } - #endif /* WOLFIP_ESP */ - { struct wolfIP_ip_packet *dispatch_ip = ip; uint32_t dispatch_len = len; @@ -8787,6 +8962,28 @@ static inline void ip_recv(struct wolfIP *s, unsigned int if_idx, iphdr_set_checksum(dispatch_ip); } + #ifdef WOLFIP_ESP + /* note: esp transport mode only handled here. + * ip forwarding would require esp tunnel mode. + * Run after the option strip above: esp_transport_unwrap reads the + * ESP header at a fixed 20-byte-IP-header offset, so it must be given + * a packet whose options have already been removed (IHL == 5). + * Otherwise an IHL>5 ESP packet has its SPI read from the option + * bytes, the SA lookup fails, and it is silently dropped. */ + if (dispatch_ip->proto == 0x32) { + int err; + if (wolfIP_ll_is_non_ethernet(s, if_idx)) { + return; + } + /* proto is ESP 0x32 (50), try to unwrap. */ + err = esp_transport_unwrap(dispatch_ip, &dispatch_len); + if (err) { + LOG("info: failed to unwrap esp packet, dropping.\n"); + return; + } + } + #endif /* WOLFIP_ESP */ + if (dispatch_ip->proto == 0x06) { struct wolfIP_tcp_seg *tcp = (struct wolfIP_tcp_seg *)dispatch_ip; tcp_input(s, if_idx, tcp, dispatch_len); @@ -8825,6 +9022,9 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin #ifdef ETHERNET struct wolfIP_ll_dev *ll; struct wolfIP_eth_frame *eth; +#if WOLFIP_PACKET_SOCKETS + int pkt_match_wildcard = 1; +#endif #else struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)buf; #endif @@ -8870,6 +9070,15 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin } if (!found) return; /* No matching VLAN sub-interface; drop. */ +#if WOLFIP_PACKET_SOCKETS + /* Tap the physical (parent) interface with the original, still-tagged + * frame so AF_PACKET listeners bound to the parent - and wildcard + * sniffers/IDS - observe VLAN traffic in wire form. The post-demux + * delivery below then targets only sockets bound explicitly to the + * sub-interface, so wildcard sockets are not given the frame twice. */ + packet_try_recv(s, if_idx, eth, len, 1); + pkt_match_wildcard = 0; +#endif /* Strip the 4-byte tag in place: slide MAC headers forward. */ memmove((uint8_t *)buf + WOLFIP_VLAN_TAG_LEN, buf, 12); buf = (uint8_t *)buf + WOLFIP_VLAN_TAG_LEN; @@ -8881,7 +9090,7 @@ static void wolfIP_recv_on(struct wolfIP *s, unsigned int if_idx, void *buf, uin } #endif /* WOLFIP_VLAN */ #if WOLFIP_PACKET_SOCKETS - packet_try_recv(s, if_idx, eth, len); + packet_try_recv(s, if_idx, eth, len, pkt_match_wildcard); #endif if (eth->type == ee16(ETH_TYPE_IP)) { struct wolfIP_ip_packet *ip = (struct wolfIP_ip_packet *)eth; @@ -9396,6 +9605,11 @@ int nslookup(struct wolfIP *s, const char *dname, uint16_t *id, void (*lookup_cb { if (!s || !dname || !id || !lookup_cb) return -22; + /* Reject before touching shared callback/type state: dns_send_query's + * busy-guard runs too late to stop an in-flight query's callback from + * being clobbered by this (rejected) call. */ + if (s->dns_id != 0) + return -16; s->dns_lookup_cb = lookup_cb; s->dns_ptr_cb = NULL; s->dns_query_type = DNS_QUERY_TYPE_A; @@ -9409,6 +9623,9 @@ int wolfIP_dns_ptr_lookup(struct wolfIP *s, uint32_t ip, uint16_t *id, void (*lo return -22; if (dns_format_ptr_name(ptr_name, sizeof(ptr_name), ip) < 0) return -22; + /* Reject before touching shared callback/type state (see nslookup). */ + if (s->dns_id != 0) + return -16; s->dns_ptr_cb = lookup_cb; s->dns_lookup_cb = NULL; s->dns_ptr_name[0] = DNS_NAME_TERMINATOR; @@ -9888,7 +10105,12 @@ int wolfIP_poll(struct wolfIP *s, uint64_t now) tx_if = 0; if (wolfIP_filter_notify_eth(WOLFIP_FILT_SENDING, s, tx_if, (struct wolfIP_eth_frame *)frame, desc->len) != 0) { - desc = fifo_next(&p->txbuf, desc); + /* The filter vetoed this frame. fifo_pop() only removes the + * tail, so walking forward with fifo_next() here would later + * pop the wrong descriptor and re-send an accepted frame. Drop + * the blocked frame from the head and re-evaluate. */ + fifo_pop(&p->txbuf); + desc = fifo_peek(&p->txbuf); continue; } wolfIP_ll_send_frame(s, tx_if, frame, desc->len);