From d5b80ccc72ed36367ac327748be66323c858ad5d Mon Sep 17 00:00:00 2001 From: David Gibson Date: Sat, 24 Sep 2022 19:08:22 +1000 Subject: Fix widespread off-by-one error dealing with port numbers Port numbers (for both TCP and UDP) are 16-bit, and so fit exactly into a 'short'. USHRT_MAX is therefore the maximum port number and this is widely used in the code. Unfortunately, a lot of those places don't actually want the maximum port number (USHRT_MAX == 65535), they want the total number of ports (65536). This leads to a number of potentially nasty consequences: * We have buffer overruns on the port_fwd::delta array if we try to use port 65535 * We have similar potential overruns for the tcp_sock_* arrays * Interestingly udp_act had the correct size, but we can calculate it in a more direct manner * We have a logical overrun of the ports bitmap as well, although it will just use an unused bit in the last byte so isnt harmful * Many loops don't consider port 65535 (which does mitigate some but not all of the buffer overruns above) * In udp_invert_portmap() we incorrectly compute the reverse port translation for return packets Correct all these by using a new NUM_PORTS defined explicitly for this purpose. Signed-off-by: David Gibson --- tcp.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tcp.c') diff --git a/tcp.c b/tcp.c index d96232c..e45dfda 100644 --- a/tcp.c +++ b/tcp.c @@ -547,9 +547,9 @@ static const char *tcp_flag_str[] __attribute((__unused__)) = { }; /* Listening sockets, used for automatic port forwarding in pasta mode only */ -static int tcp_sock_init_lo [USHRT_MAX][IP_VERSIONS]; -static int tcp_sock_init_ext [USHRT_MAX][IP_VERSIONS]; -static int tcp_sock_ns [USHRT_MAX][IP_VERSIONS]; +static int tcp_sock_init_lo [NUM_PORTS][IP_VERSIONS]; +static int tcp_sock_init_ext [NUM_PORTS][IP_VERSIONS]; +static int tcp_sock_ns [NUM_PORTS][IP_VERSIONS]; /* Table of destinations with very low RTT (assumed to be local), LRU */ static struct in6_addr low_rtt_dst[LOW_RTT_TABLE_SIZE]; @@ -3186,7 +3186,7 @@ static int tcp_sock_init_ns(void *arg) ns_enter(c); - for (port = 0; port < USHRT_MAX; port++) { + for (port = 0; port < NUM_PORTS; port++) { if (!bitmap_isset(c->tcp.fwd_out.map, port)) continue; @@ -3386,7 +3386,7 @@ static int tcp_port_rebind(void *arg) if (a->bind_in_ns) { ns_enter(a->c); - for (port = 0; port < USHRT_MAX; port++) { + for (port = 0; port < NUM_PORTS; port++) { if (!bitmap_isset(a->c->tcp.fwd_out.map, port)) { if (tcp_sock_ns[port][V4] >= 0) { close(tcp_sock_ns[port][V4]); @@ -3410,7 +3410,7 @@ static int tcp_port_rebind(void *arg) tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, port); } } else { - for (port = 0; port < USHRT_MAX; port++) { + for (port = 0; port < NUM_PORTS; port++) { if (!bitmap_isset(a->c->tcp.fwd_in.map, port)) { if (tcp_sock_init_ext[port][V4] >= 0) { close(tcp_sock_init_ext[port][V4]); -- cgit v1.2.3