From f829a5cbfa53c8536c759d0c28e2e189e95df0ae Mon Sep 17 00:00:00 2001 From: Tinic Uro Date: Sun, 26 Jul 2026 16:23:21 -0700 Subject: [PATCH 1/2] tcp: remove bias from the randomised initial sequence number _nxd_tcp_client_socket_connect() and _nx_tcp_server_socket_accept() compose a fresh ISN from two random draws with a bitwise OR: socket_ptr -> nx_tcp_socket_tx_sequence = (((ULONG)NX_RAND()) << NX_SHIFT_BY_16) & 0xFFFFFFFF; socket_ptr -> nx_tcp_socket_tx_sequence |= (ULONG)NX_RAND(); The two draws overlap in bits 16-30, and an OR of two random bits is 1 with probability 3/4. rand() returns 0..RAND_MAX, so with the usual RAND_MAX of 0x7FFFFFFF the ISN has 17 bits at P(1) = 1/2 and 15 bits at P(1) = 3/4. That is 29.2 bits of Shannon entropy and 17 + 15*log2(4/3) = 23.2 bits of min-entropy: the likeliest ISN comes up 2^(32-23.2) = 438 times more often than under a uniform distribution, and an attacker who enumerates the dense upper values first faces 2^23 rather than 2^32. This is a property of the composition, not of the generator. A port that points NX_RAND at a CSPRNG loses the same nine bits, because the loss is entirely in the overlap between the two draws. Adding instead of ORing removes it, and matches the else branch immediately below -- the path every reused socket takes, which already adds. With a 31-bit draw the sum is exactly uniform over the whole 32-bit space: (a << 16) is uniform over the 65536 multiples of 65536, and adding an independent uniform value over [0, 2^31) leaves every 32-bit residue equally likely. Measured over 2e7 samples of a 31-bit uniform source: Shannon min-entropy likeliest ISN vs uniform with | 29.168 23.220 440x with + 32.000 31.991 1x The consequence is blind in-window injection: an off-path attacker needs a sequence number inside the receive window to land a spoofed RST or segment, and nine bits of bias is nine bits fewer guesses. RFC 6528 asks that an ISN not be predictable. This does not implement 6528's four-tuple hash; it only removes a bias in a value the code already meant to be random. Found while auditing SYN traces from an m68k AmigaOS port: bits 16-30 were set in 35 of 45 SYNs (0.78, predicted 0.75) before the change and 69 of 135 (0.51, predicted 0.50) after. Signed-off-by: Tinic Uro --- common/src/nx_tcp_server_socket_accept.c | 2 +- common/src/nxd_tcp_client_socket_connect.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/src/nx_tcp_server_socket_accept.c b/common/src/nx_tcp_server_socket_accept.c index 04804a61e..23deccc9a 100644 --- a/common/src/nx_tcp_server_socket_accept.c +++ b/common/src/nx_tcp_server_socket_accept.c @@ -106,7 +106,7 @@ NX_IP *ip_ptr; if (socket_ptr -> nx_tcp_socket_tx_sequence == 0) { socket_ptr -> nx_tcp_socket_tx_sequence = (((ULONG)NX_RAND()) << NX_SHIFT_BY_16) & 0xFFFFFFFF; - socket_ptr -> nx_tcp_socket_tx_sequence |= (ULONG)NX_RAND(); + socket_ptr -> nx_tcp_socket_tx_sequence += (ULONG)NX_RAND(); } else { diff --git a/common/src/nxd_tcp_client_socket_connect.c b/common/src/nxd_tcp_client_socket_connect.c index 56b68f884..56eb3ad03 100644 --- a/common/src/nxd_tcp_client_socket_connect.c +++ b/common/src/nxd_tcp_client_socket_connect.c @@ -411,7 +411,7 @@ ULONG ip_address_log = 0; if (socket_ptr -> nx_tcp_socket_tx_sequence == 0) { socket_ptr -> nx_tcp_socket_tx_sequence = (((ULONG)NX_RAND()) << NX_SHIFT_BY_16) & 0xFFFFFFFF; - socket_ptr -> nx_tcp_socket_tx_sequence |= (ULONG)NX_RAND(); + socket_ptr -> nx_tcp_socket_tx_sequence += (ULONG)NX_RAND(); } else { From 2382dcbaed2654b788229a7c82d415613f63dad0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Desbiens?= Date: Thu, 13 Aug 2026 09:32:48 -0400 Subject: [PATCH 2/2] Masked the initial sequence number after the addition, not only before The first line of each pair carries an explicit & 0xFFFFFFFF, which documents the intent to keep the sequence number inside 32 bits. While the second line was |=, that mask could not be violated. Now that it is +=, staying inside 32 bits depends on ULONG being exactly 32 bits so the wrap does the masking implicitly. That holds on every port today, ThreadX pinning ULONG to 32 bits everywhere, but it is an implicit dependency where the line above states an explicit one, and the mask on the first line would otherwise read as protecting something it no longer protects. Mask the result as well, at both sites. On a 32-bit ULONG this is a no-op the compiler discards; on a hypothetical port with a wider ULONG it keeps the invariant the first line already claims. No behaviour change. Verified that the library builds and that three regression tests which read nx_tcp_socket_tx_sequence still pass: netx_tcp_4_duplicate_ack_test, netx_tcp_new_reno_algorithm_test2 and netx_tcp_overlapping_packet_test_2. A two-million-draw model of the three lines with a 32-bit ULONG gives a mean of 2146460131 against the uniform 2147483648, so the distribution the PR set out to fix is unaffected. Assisted-by: Claude Code (Opus 5) --- common/src/nx_tcp_server_socket_accept.c | 1 + common/src/nxd_tcp_client_socket_connect.c | 1 + 2 files changed, 2 insertions(+) diff --git a/common/src/nx_tcp_server_socket_accept.c b/common/src/nx_tcp_server_socket_accept.c index 23deccc9a..e0059cd9d 100644 --- a/common/src/nx_tcp_server_socket_accept.c +++ b/common/src/nx_tcp_server_socket_accept.c @@ -107,6 +107,7 @@ NX_IP *ip_ptr; { socket_ptr -> nx_tcp_socket_tx_sequence = (((ULONG)NX_RAND()) << NX_SHIFT_BY_16) & 0xFFFFFFFF; socket_ptr -> nx_tcp_socket_tx_sequence += (ULONG)NX_RAND(); + socket_ptr -> nx_tcp_socket_tx_sequence &= 0xFFFFFFFF; } else { diff --git a/common/src/nxd_tcp_client_socket_connect.c b/common/src/nxd_tcp_client_socket_connect.c index 56eb3ad03..9c734ece8 100644 --- a/common/src/nxd_tcp_client_socket_connect.c +++ b/common/src/nxd_tcp_client_socket_connect.c @@ -412,6 +412,7 @@ ULONG ip_address_log = 0; { socket_ptr -> nx_tcp_socket_tx_sequence = (((ULONG)NX_RAND()) << NX_SHIFT_BY_16) & 0xFFFFFFFF; socket_ptr -> nx_tcp_socket_tx_sequence += (ULONG)NX_RAND(); + socket_ptr -> nx_tcp_socket_tx_sequence &= 0xFFFFFFFF; } else {