diff --git a/src/port/stm32h563/ssh_server.h b/src/port/stm32h563/ssh_server.h index 1a77085c..b33231e4 100644 --- a/src/port/stm32h563/ssh_server.h +++ b/src/port/stm32h563/ssh_server.h @@ -35,7 +35,9 @@ int ssh_server_init(struct wolfIP *stack, uint16_t port, ssh_debug_cb debug); * Returns 0 on success */ int ssh_server_poll(void); -/* Get SSH server uptime in seconds (for status display) */ +/* Get SSH server uptime in seconds (for status display). + * Currently a placeholder: returns 0 until a main-loop tick source is + * integrated, so the "uptime" SSH command always reports zero. */ uint32_t ssh_server_get_uptime(void); #endif /* SSH_SERVER_H */ diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c index 3fc8ce70..ec9e9ee2 100644 --- a/src/test/unit/unit.c +++ b/src/test/unit/unit.c @@ -649,6 +649,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_utils, test_tcp_fin_wait_1_to_closing); tcase_add_test(tc_utils, test_tcp_last_ack_closes_socket); tcase_add_test(tc_utils, test_tcp_last_ack_closes_socket_delivers_closed_event); + tcase_add_test(tc_utils, test_handle_socket_callbacks_keeps_recreated_socket); tcase_add_test(tc_utils, test_tcp_last_ack_partial_ack_keeps_socket_and_timer); tcase_add_test(tc_utils, test_tcp_ack_acks_data_and_sets_writable); tcase_add_test(tc_utils, test_tcp_ack_duplicate_resend_clears_sent); @@ -921,6 +922,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_proto, test_regression_icmp_echo_request_non_local_dst_no_reply); tcase_add_test(tc_proto, test_tcp_listen_rejects_wrong_interface); tcase_add_test(tc_proto, test_tcp_listen_accepts_bound_interface); + tcase_add_test(tc_proto, test_tcp_listen_requires_matching_local_ip); tcase_add_test(tc_proto, test_tcp_listen_accepts_any_interface); tcase_add_test(tc_proto, test_sock_connect_selects_local_ip_multi_if); tcase_add_test(tc_proto, test_icmp_socket_send_recv); @@ -1148,6 +1150,9 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_icmp_input_echo_reply_path_filter_at_eth); tcase_add_test(tc_core, test_ip_recv_with_options_oversize_dropped); tcase_add_test(tc_core, test_wolfip_recv_on_null_stack_returns); +#if WOLFIP_RAWSOCKETS + tcase_add_test(tc_core, test_raw_sendto_rejects_oversized_len_before_narrowing); +#endif /* Socket API arms: TCP, RAW, PACKET */ tcase_add_test(tc_core, test_register_callback_tcp_stores_handle); @@ -1529,6 +1534,7 @@ Suite *wolf_suite(void) tcase_add_test(tc_core, test_dns_callback_rcode_nonzero_aborts_query); tcase_add_test(tc_core, test_dns_callback_zero_ancount_no_delivery); tcase_add_test(tc_core, test_dns_callback_aaaa_answer_skipped_for_a_query); + tcase_add_test(tc_core, test_dns_callback_qr_without_rd_is_accepted); tcase_add_test(tc_core, test_dns_callback_rr_rdlen_truncated_aborts_query); tcase_add_test(tc_core, test_dns_callback_bad_question_name_aborts_query); tcase_add_test(tc_core, test_dns_callback_answer_forward_ptr_aborts_query); diff --git a/src/test/unit/unit_tests_branches.c b/src/test/unit/unit_tests_branches.c index 64f616cb..585dd701 100644 --- a/src/test/unit/unit_tests_branches.c +++ b/src/test/unit/unit_tests_branches.c @@ -2607,3 +2607,37 @@ START_TEST(test_wolfip_recv_on_null_stack_returns) wolfIP_recv_on(NULL, TEST_PRIMARY_IF, buf, sizeof(buf)); } END_TEST + +#if WOLFIP_RAWSOCKETS +/* F-8525: the raw-socket sendto path must reject a payload that cannot fit + * in a frame before narrowing len to uint32_t. A size_t len above the + * LINK_MTU-derived bound wraps in the total_len computation, slips past the + * LINK_MTU guard, and lets the payload memcpy overflow the fixed-size frame + * buffer. len = UINT32_MAX + 100 narrows to 100 (which would pass the MTU + * guard) but must be rejected before any memcpy. The overflow itself cannot + * be exercised in a unit test (it needs a >4GB buffer), so this pins the + * new size_t bound: the oversized length is refused, not narrowed. */ +START_TEST(test_raw_sendto_rejects_oversized_len_before_narrowing) +{ + struct wolfIP s; + int fd; + uint8_t buf[8]; + struct wolfIP_sockaddr_in sin; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0); + + fd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_RAW, 0); + ck_assert_int_ge(fd, 0); + + memset(&sin, 0, sizeof(sin)); + sin.sin_family = AF_INET; + sin.sin_addr.s_addr = ee32(0x0A000002U); + + ck_assert_int_eq(wolfIP_sock_sendto(&s, fd, buf, (size_t)UINT32_MAX + 100, 0, + (struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EINVAL); +} +END_TEST +#endif /* WOLFIP_RAWSOCKETS */ + diff --git a/src/test/unit/unit_tests_dns_edges.c b/src/test/unit/unit_tests_dns_edges.c index f467d875..6423577d 100644 --- a/src/test/unit/unit_tests_dns_edges.c +++ b/src/test/unit/unit_tests_dns_edges.c @@ -196,6 +196,57 @@ START_TEST(test_dns_callback_aaaa_answer_skipped_for_a_query) } END_TEST +/* ------------------------------------------------------------------ * + * F-6211: response detection must key on the QR bit alone (RFC 1035 + * s4.1.1). A conformant server that does not echo the RD bit must still + * have its reply parsed. Requiring RD as well silently drops such a + * response and lets the query time out and retransmit. Flags here are + * 0x8000 (QR set, RD clear). + * ------------------------------------------------------------------ */ +START_TEST(test_dns_callback_qr_without_rd_is_accepted) +{ + struct wolfIP s; + uint8_t response[128]; + int pos; + struct dns_rr *rr; + uint8_t a_rdata[4] = {0x0A, 0x00, 0x00, 0x02}; + + wolfIP_init(&s); + mock_link_init(&s); + s.dns_server = 0x0A000001U; + arm_dns_query(&s, 0x3333, dns_qname_example_com, + (int)sizeof(dns_qname_example_com), DNS_A); + dns_lookup_calls = 0; + dns_lookup_ip = 0; + s.dns_lookup_cb = test_dns_lookup_cb; + s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP); + ck_assert_int_gt(s.dns_udp_sd, 0); + + /* QR set, RD clear, RCODE 0, TC clear. */ + pos = build_dns_a_response_header(response, sizeof(response), + s.dns_id, 0x8000, 1, 1, NULL); + /* Answer NAME: compressed pointer to the question name. */ + 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((uint16_t)sizeof(a_rdata)); + pos += (int)sizeof(struct dns_rr); + memcpy(&response[pos], a_rdata, sizeof(a_rdata)); + pos += (int)sizeof(a_rdata); + + 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); + + /* The QR-only response must be parsed and the lookup delivered. */ + ck_assert_int_eq(dns_lookup_calls, 1); + ck_assert_uint_eq(dns_lookup_ip, 0x0A000002U); +} +END_TEST + /* ------------------------------------------------------------------ * * dns_callback: answer rdlen advertised larger than remaining buffer * → abort query (line 8997-8999) diff --git a/src/test/unit/unit_tests_proto.c b/src/test/unit/unit_tests_proto.c index ee4ed934..683c576d 100644 --- a/src/test/unit/unit_tests_proto.c +++ b/src/test/unit/unit_tests_proto.c @@ -4811,6 +4811,53 @@ START_TEST(test_tcp_listen_accepts_bound_interface) } END_TEST +/* F-10281: a listener bound to a specific local address must only match + * segments addressed to that address. The SYN path already validates + * bound_local_ip, but a non-SYN segment for the same port and a different + * local address on the same host overwrites the listener's + * if_idx/last_pkt_ttl/peer_rwnd and sets matched (suppressing the RFC 793 + * unmatched RST) before any address validation. A segment for the bound + * address must still match, so the check is not over-restricting. */ +START_TEST(test_tcp_listen_requires_matching_local_ip) +{ + struct wolfIP s; + const ip4 primary_ip = 0xC0A80002U; + const ip4 secondary_ip = 0xC0A80101U; + const uint16_t listen_port = 23456; + int listen_fd; + struct wolfIP_sockaddr_in addr; + struct tsocket *listener; + uint8_t ttl_before; + + setup_stack_with_two_ifaces(&s, primary_ip, secondary_ip); + + listen_fd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, 0); + ck_assert_int_ge(listen_fd, 0); + listener = &s.tcpsockets[SOCKET_UNMARK(listen_fd)]; + + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_port = ee16(listen_port); + addr.sin_addr.s_addr = ee32(secondary_ip); + ck_assert_int_eq(wolfIP_sock_bind(&s, listen_fd, (struct wolfIP_sockaddr *)&addr, sizeof(addr)), 0); + ck_assert_int_eq(wolfIP_sock_listen(&s, listen_fd, 1), 0); + ck_assert_uint_eq(listener->bound_local_ip, secondary_ip); + ck_assert_int_eq(listener->sock.tcp.state, TCP_LISTEN); + ttl_before = listener->last_pkt_ttl; + + /* (1) A non-SYN segment for the same port but a different local address + * must not mutate the listener's bookkeeping. */ + inject_tcp_segment(&s, TEST_PRIMARY_IF, 0x0A0000A1U, primary_ip, 40000, + listen_port, 100, 0, 0); + ck_assert_uint_eq(listener->last_pkt_ttl, ttl_before); + + /* (2) A segment for the bound address still matches (not over-restricted). */ + inject_tcp_segment(&s, TEST_SECOND_IF, 0x0A0000A2U, secondary_ip, 40000, + listen_port, 200, 0, 0); + ck_assert_uint_eq(listener->last_pkt_ttl, 64); +} +END_TEST + START_TEST(test_tcp_listen_accepts_any_interface) { struct wolfIP s; diff --git a/src/test/unit/unit_tests_tcp_ack.c b/src/test/unit/unit_tests_tcp_ack.c index 1e749ec5..c4b529da 100644 --- a/src/test/unit/unit_tests_tcp_ack.c +++ b/src/test/unit/unit_tests_tcp_ack.c @@ -4029,6 +4029,60 @@ START_TEST(test_tcp_last_ack_closes_socket_delivers_closed_event) } END_TEST +/* F-8523: the post-callback reap in handle_socket_callbacks must not destroy + * a socket that the close callback closed and re-created in the same slot. + * The reap historically keyed on the slot's TCP_CLOSED state, which a fresh + * socket has by design, so it must be gated on the slot still holding the + * dispatched socket (same callback pair). */ +static int test_f8523_recreated_fd; +static void test_f8523_close_recreate_cb(int fd, uint16_t events, void *arg) +{ + struct wolfIP *s = (struct wolfIP *)arg; + (void)events; + /* Close the socket (frees its slot) and create a fresh one in its place. */ + (void)wolfIP_sock_close(s, fd); + test_f8523_recreated_fd = wolfIP_sock_socket(s, AF_INET, IPSTACK_SOCK_STREAM, 0); +} + +START_TEST(test_handle_socket_callbacks_keeps_recreated_socket) +{ + struct wolfIP s; + struct tsocket *ts; + ip4 local_ip = 0x0A000001U; + uint16_t local_port = 6669; + + wolfIP_init(&s); + mock_link_init(&s); + wolfIP_ipconfig_set(&s, local_ip, 0xFFFFFF00U, 0); + + /* Put slot 0 in the RX-deferred close state: TCP_CLOSED with a pending + * CB_EVENT_CLOSED and an armed callback, no close_notify_pending. */ + ts = &s.tcpsockets[0]; + memset(ts, 0, sizeof(*ts)); + ts->proto = WI_IPPROTO_TCP; + ts->S = &s; + ts->sock.tcp.state = TCP_CLOSED; + ts->local_ip = local_ip; + ts->src_port = local_port; + ts->callback = test_f8523_close_recreate_cb; + ts->callback_arg = &s; + ts->events = CB_EVENT_CLOSED; + queue_init(&ts->sock.tcp.rxbuf, ts->rxmem, RXBUF_SIZE, ts->sock.tcp.ack); + + test_f8523_recreated_fd = -1; + + /* poll Step 3 dispatches the deferred CB_EVENT_CLOSED; the callback closes + * the socket and re-creates a fresh one in the same slot. */ + (void)wolfIP_poll(&s, 1); + + /* The callback ran and created a fresh socket. */ + ck_assert_int_ge(test_f8523_recreated_fd, 0); + /* The fresh socket must have survived the dispatcher's post-callback reap. */ + ck_assert_uint_eq(s.tcpsockets[SOCKET_UNMARK(test_f8523_recreated_fd)].proto, + (uint8_t)WI_IPPROTO_TCP); +} +END_TEST + START_TEST(test_tcp_last_ack_partial_ack_keeps_socket_and_timer) { struct wolfIP s; diff --git a/src/tftp/wolftftp.h b/src/tftp/wolftftp.h index 085efea7..5397ff15 100644 --- a/src/tftp/wolftftp.h +++ b/src/tftp/wolftftp.h @@ -67,9 +67,11 @@ /* Worst-case RRQ/WRQ on the wire: * opcode(2) + filename(MAX_FILENAME, null-terminated) + "octet\0"(6) - * + blksize/value(13) + timeout/value(12) + windowsize/value(13) - * + tsize/value(17) = 63 + MAX_FILENAME. The constant below adds a - * generous margin so future options do not silently truncate. */ + * + blksize/value(13) + timeout/value(14) + windowsize/value(13) + * + tsize/value(17) = 65 + MAX_FILENAME. The constant below adds a + * generous margin so future options do not silently truncate. The timeout + * value is the widest: timeout_s is an unclamped uint16_t, so 65535 serializes + * as "65535\0" (6 bytes) behind "timeout\0" (8 bytes). */ #define WOLFTFTP_REQ_BUF_MAX (WOLFTFTP_MAX_FILENAME + 128U) #define WOLFTFTP_ERR_IO (-1000) diff --git a/src/wolfip.c b/src/wolfip.c index 3951be7f..4bda69a7 100644 --- a/src/wolfip.c +++ b/src/wolfip.c @@ -2707,7 +2707,6 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, return; for (i = 0; i < MAX_UDPSOCKETS; i++) { struct tsocket *t = &s->udpsockets[i]; - uint32_t expected_len; /* Only connected UDP sockets restrict by the peer's * ip/port. Unconnected sockets (sendto-only or pure listeners) * must accept datagrams from any source, per POSIX. This is @@ -2733,13 +2732,10 @@ static void udp_try_recv(struct wolfIP *s, unsigned int if_idx, if (t->local_ip == 0) t->if_idx = (uint8_t)if_idx; - /* UDP datagram sanity checks */ - /* Allow some tolerance for padding/alignment (up to 4 bytes) */ - expected_len = ee16(udp->len) + IP_HEADER_LEN + ETH_HEADER_LEN; - if ((int)frame_len < (int)expected_len) - return; /* A bound socket matched this datagram. If the RX FIFO is full, - * drop silently instead of misreporting the port as closed. */ + * drop silently instead of misreporting the port as closed. + * (The frame_len vs declared UDP length bound is already + * enforced by the unconditional guard before the socket loop.) */ matched = 1; if (fifo_push(&t->sock.udp.rxbuf, udp, frame_len) == 0) { t->last_pkt_ttl = udp->ip.ttl; @@ -5433,6 +5429,23 @@ static void tcp_input(struct wolfIP *S, unsigned int if_idx, /* Not the right local endpoint */ continue; } + } else { + /* LISTEN: a specifically-bound listener (bound_local_ip != + * 0.0.0.0) must only match segments addressed to its bound + * address; a wildcard listener accepts any local address. + * The SYN path already enforces this for SYNs, but without it + * here a non-SYN segment for a different local IP on the same + * host overwrites the listener's if_idx/last_pkt_ttl/peer_rwnd + * and sets matched, which corrupts listener MTU/TTL + * bookkeeping and suppresses the RFC 793 unmatched RST. + * bound_local_ip (not local_ip) is the discriminator: a + * 0.0.0.0 bind leaves local_ip set to the interface/primary + * address as a default source. */ + if (t->bound_local_ip != IPADDR_ANY && + t->bound_local_ip != ee32(tcp->ip.dst)) { + /* Not the right local endpoint */ + continue; + } } t->if_idx = (uint8_t)if_idx; t->last_pkt_ttl = tcp->ip.ttl; @@ -6860,6 +6873,12 @@ int wolfIP_sock_sendto(struct wolfIP *s, int sockfd, const void *buf, size_t len return -WOLFIP_EINVAL; if (len == 0) return -WOLFIP_EINVAL; + /* Reject payloads that cannot fit in a frame before narrowing len to + * uint32_t below: a size_t len above the LINK_MTU-derived bound wraps + * in the total_len computation, slips past the LINK_MTU guard, and + * lets the payload memcpy overflow the fixed-size frame buffer. */ + if (len > (size_t)LINK_MTU) + return -WOLFIP_EINVAL; if (sin) { if (addrlen < sizeof(struct wolfIP_sockaddr_in)) return -WOLFIP_EINVAL; @@ -10616,7 +10635,11 @@ void wolfIP_recv_ex(struct wolfIP *s, unsigned int if_idx, void *buf, uint32_t l #define DNS_RD 0x0100 /* Recursion desired */ #define DNS_TC 0x0200 /* Truncated response */ #define DNS_RCODE_MASK 0x000F -#define DNS_FLAGS_RESPONSE_RD (DNS_RD | ((uint16_t)DNS_RESPONSE << 8)) +/* QR bit (bit 15 of the 16-bit flags field): per RFC 1035 s4.1.1 this alone + * distinguishes a response from a query. RD is only the Recursion-Desired + * flag echoed from the query, so it must not gate response detection. */ +#define DNS_FLAGS_RESPONSE ((uint16_t)DNS_RESPONSE << 8) +#define DNS_FLAGS_RESPONSE_RD (DNS_RD | DNS_FLAGS_RESPONSE) #define DNS_ID_NONE 0 #define DNS_QUESTION_COUNT 1 #define DNS_MIN_ID 1 @@ -10945,8 +10968,11 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg) if (ee16(hdr->id) != s->dns_id) return; flags = ee16(hdr->flags); - /* Parse DNS response */ - if ((flags & DNS_FLAGS_RESPONSE_RD) == DNS_FLAGS_RESPONSE_RD) { + /* Parse DNS response: key on the QR bit alone (RFC 1035 s4.1.1). A + * conformant server that does not echo the RD bit must still have its + * reply parsed; requiring RD as well silently drops such responses + * and lets the outstanding query time out and retransmit. */ + if ((flags & DNS_FLAGS_RESPONSE) != 0) { if ((flags & DNS_TC) != 0) { dns_abort_query(s); return; @@ -11245,20 +11271,30 @@ static void handle_socket_callbacks(struct wolfIP *s) if ((ts->sock.tcp.state == TCP_CLOSED) && !(ts->events & CB_EVENT_CLOSED)) continue; { + tsocket_cb cb = ts->callback; + void *cb_arg = ts->callback_arg; uint16_t events = ts->events; ts->events = 0; - ts->callback(i | MARK_TCP_SOCKET, events, ts->callback_arg); - } - - /* Now that CB_EVENT_CLOSED has been delivered, reap the deferred-close - * socket. Disarm the callback first so close_socket() takes the plain - * teardown path instead of re-deferring (it re-arms close_notify_pending - * whenever a TCP socket still has a callback). A socket closed elsewhere - * is already memset (callback NULL) and never reaches this branch. */ - if (ts->sock.tcp.state == TCP_CLOSED) { - ts->callback = NULL; - ts->callback_arg = NULL; - close_socket(ts); + cb(i | MARK_TCP_SOCKET, events, cb_arg); + + /* Now that CB_EVENT_CLOSED has been delivered, reap the + * deferred-close socket - but only if the slot still holds the + * socket that was just dispatched. The callback may have closed + * this socket (freeing the slot) and allocated a fresh one in its + * place; a fresh socket is TCP_CLOSED by design, so reaping on + * state alone would destroy it. A replaced slot carries a + * different (or no) callback, so key the reap on the identity of + * the callback pair. Disarm the callback first so close_socket() + * takes the plain teardown path instead of re-deferring (it + * re-arms close_notify_pending whenever a TCP socket still has a + * callback). A socket closed elsewhere is already memset (callback + * NULL) and never reaches this branch. */ + if ((ts->callback == cb) && (ts->callback_arg == cb_arg) && + (ts->sock.tcp.state == TCP_CLOSED)) { + ts->callback = NULL; + ts->callback_arg = NULL; + close_socket(ts); + } } }