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 <david(a)gibson.dropbear.id.au>
---
conf.c | 4 ++--
port_fwd.h | 7 +++++--
tcp.c | 12 ++++++------
udp.c | 10 +++++-----
udp.h | 2 +-
5 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/conf.c b/conf.c
index 503d894..5cdfe31 100644
--- a/conf.c
+++ b/conf.c
@@ -209,7 +209,7 @@ static int conf_ports(const struct ctx *c, char optname, const char
*optarg,
if (sep == p)
break;
- if (port > USHRT_MAX || errno)
+ if (port >= NUM_PORTS || errno)
goto bad;
switch (*sep) {
@@ -276,7 +276,7 @@ static int conf_ports(const struct ctx *c, char optname, const char
*optarg,
if (sep == p)
break;
- if (port > USHRT_MAX || errno)
+ if (port >= NUM_PORTS || errno)
goto bad;
/* -p 22
diff --git a/port_fwd.h b/port_fwd.h
index 5507bf2..976138f 100644
--- a/port_fwd.h
+++ b/port_fwd.h
@@ -7,6 +7,9 @@
#ifndef PORT_FWD_H
#define PORT_FWD_H
+/* Number of ports for both TCP and UDP */
+#define NUM_PORTS (1U << 16)
+
enum port_fwd_mode {
FWD_SPEC = 1,
FWD_NONE,
@@ -14,7 +17,7 @@ enum port_fwd_mode {
FWD_ALL,
};
-typedef uint8_t port_fwd_map[DIV_ROUND_UP(USHRT_MAX, 8)];
+typedef uint8_t port_fwd_map[DIV_ROUND_UP(NUM_PORTS, 8)];
/**
* port_fwd - Describes port forwarding for one protocol and direction
@@ -25,7 +28,7 @@ typedef uint8_t port_fwd_map[DIV_ROUND_UP(USHRT_MAX, 8)];
struct port_fwd {
enum port_fwd_mode mode;
port_fwd_map map;
- in_port_t delta[USHRT_MAX];
+ in_port_t delta[NUM_PORTS];
};
#endif /* PORT_FWD_H */
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]);
diff --git a/udp.c b/udp.c
index 27c3aa3..bd3dc72 100644
--- a/udp.c
+++ b/udp.c
@@ -164,8 +164,8 @@ struct udp_splice_port {
};
/* Port tracking, arrays indexed by packet source port (host order) */
-static struct udp_tap_port udp_tap_map [IP_VERSIONS][USHRT_MAX];
-static struct udp_splice_port udp_splice_map [IP_VERSIONS][USHRT_MAX];
+static struct udp_tap_port udp_tap_map [IP_VERSIONS][NUM_PORTS];
+static struct udp_splice_port udp_splice_map [IP_VERSIONS][NUM_PORTS];
enum udp_act_type {
UDP_ACT_TAP,
@@ -175,7 +175,7 @@ enum udp_act_type {
};
/* Activity-based aging for bindings */
-static uint8_t udp_act[IP_VERSIONS][UDP_ACT_TYPE_MAX][(USHRT_MAX + 1) / 8];
+static uint8_t udp_act[IP_VERSIONS][UDP_ACT_TYPE_MAX][DIV_ROUND_UP(NUM_PORTS, 8)];
/* Static buffers */
@@ -271,7 +271,7 @@ static void udp_invert_portmap(struct udp_port_fwd *fwd)
in_port_t delta = fwd->f.delta[i];
if (delta)
- fwd->rdelta[(in_port_t)i + delta] = USHRT_MAX - delta;
+ fwd->rdelta[(in_port_t)i + delta] = NUM_PORTS - delta;
}
}
@@ -1198,7 +1198,7 @@ int udp_sock_init_ns(void *arg)
if (ns_enter(c))
return 0;
- for (dst = 0; dst < USHRT_MAX; dst++) {
+ for (dst = 0; dst < NUM_PORTS; dst++) {
if (!bitmap_isset(c->udp.fwd_out.f.map, dst))
continue;
diff --git a/udp.h b/udp.h
index bc7b259..d14df0a 100644
--- a/udp.h
+++ b/udp.h
@@ -50,7 +50,7 @@ union udp_epoll_ref {
*/
struct udp_port_fwd {
struct port_fwd f;
- in_port_t rdelta[USHRT_MAX];
+ in_port_t rdelta[NUM_PORTS];
};
/**
--
2.37.3