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 <david@gibson.dropbear.id.au>
This commit is contained in:
parent
3ede07aac9
commit
d5b80ccc72
5 changed files with 19 additions and 16 deletions
4
conf.c
4
conf.c
|
@ -209,7 +209,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
|
||||||
if (sep == p)
|
if (sep == p)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
if (port > USHRT_MAX || errno)
|
if (port >= NUM_PORTS || errno)
|
||||||
goto bad;
|
goto bad;
|
||||||
|
|
||||||
switch (*sep) {
|
switch (*sep) {
|
||||||
|
@ -276,7 +276,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
|
||||||
if (sep == p)
|
if (sep == p)
|
||||||
break;
|
break;
|
||||||
|
|
||||||
if (port > USHRT_MAX || errno)
|
if (port >= NUM_PORTS || errno)
|
||||||
goto bad;
|
goto bad;
|
||||||
|
|
||||||
/* -p 22
|
/* -p 22
|
||||||
|
|
|
@ -7,6 +7,9 @@
|
||||||
#ifndef PORT_FWD_H
|
#ifndef PORT_FWD_H
|
||||||
#define PORT_FWD_H
|
#define PORT_FWD_H
|
||||||
|
|
||||||
|
/* Number of ports for both TCP and UDP */
|
||||||
|
#define NUM_PORTS (1U << 16)
|
||||||
|
|
||||||
enum port_fwd_mode {
|
enum port_fwd_mode {
|
||||||
FWD_SPEC = 1,
|
FWD_SPEC = 1,
|
||||||
FWD_NONE,
|
FWD_NONE,
|
||||||
|
@ -14,7 +17,7 @@ enum port_fwd_mode {
|
||||||
FWD_ALL,
|
FWD_ALL,
|
||||||
};
|
};
|
||||||
|
|
||||||
#define PORT_BITMAP_SIZE DIV_ROUND_UP(USHRT_MAX, 8)
|
#define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* port_fwd - Describes port forwarding for one protocol and direction
|
* port_fwd - Describes port forwarding for one protocol and direction
|
||||||
|
@ -25,7 +28,7 @@ enum port_fwd_mode {
|
||||||
struct port_fwd {
|
struct port_fwd {
|
||||||
enum port_fwd_mode mode;
|
enum port_fwd_mode mode;
|
||||||
uint8_t map[PORT_BITMAP_SIZE];
|
uint8_t map[PORT_BITMAP_SIZE];
|
||||||
in_port_t delta[USHRT_MAX];
|
in_port_t delta[NUM_PORTS];
|
||||||
};
|
};
|
||||||
|
|
||||||
#endif /* PORT_FWD_H */
|
#endif /* PORT_FWD_H */
|
||||||
|
|
12
tcp.c
12
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 */
|
/* 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_lo [NUM_PORTS][IP_VERSIONS];
|
||||||
static int tcp_sock_init_ext [USHRT_MAX][IP_VERSIONS];
|
static int tcp_sock_init_ext [NUM_PORTS][IP_VERSIONS];
|
||||||
static int tcp_sock_ns [USHRT_MAX][IP_VERSIONS];
|
static int tcp_sock_ns [NUM_PORTS][IP_VERSIONS];
|
||||||
|
|
||||||
/* Table of destinations with very low RTT (assumed to be local), LRU */
|
/* Table of destinations with very low RTT (assumed to be local), LRU */
|
||||||
static struct in6_addr low_rtt_dst[LOW_RTT_TABLE_SIZE];
|
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);
|
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))
|
if (!bitmap_isset(c->tcp.fwd_out.map, port))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
|
@ -3386,7 +3386,7 @@ static int tcp_port_rebind(void *arg)
|
||||||
if (a->bind_in_ns) {
|
if (a->bind_in_ns) {
|
||||||
ns_enter(a->c);
|
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 (!bitmap_isset(a->c->tcp.fwd_out.map, port)) {
|
||||||
if (tcp_sock_ns[port][V4] >= 0) {
|
if (tcp_sock_ns[port][V4] >= 0) {
|
||||||
close(tcp_sock_ns[port][V4]);
|
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);
|
tcp_sock_init(a->c, 1, AF_UNSPEC, NULL, port);
|
||||||
}
|
}
|
||||||
} else {
|
} 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 (!bitmap_isset(a->c->tcp.fwd_in.map, port)) {
|
||||||
if (tcp_sock_init_ext[port][V4] >= 0) {
|
if (tcp_sock_init_ext[port][V4] >= 0) {
|
||||||
close(tcp_sock_init_ext[port][V4]);
|
close(tcp_sock_init_ext[port][V4]);
|
||||||
|
|
10
udp.c
10
udp.c
|
@ -164,8 +164,8 @@ struct udp_splice_port {
|
||||||
};
|
};
|
||||||
|
|
||||||
/* Port tracking, arrays indexed by packet source port (host order) */
|
/* 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_tap_port udp_tap_map [IP_VERSIONS][NUM_PORTS];
|
||||||
static struct udp_splice_port udp_splice_map [IP_VERSIONS][USHRT_MAX];
|
static struct udp_splice_port udp_splice_map [IP_VERSIONS][NUM_PORTS];
|
||||||
|
|
||||||
enum udp_act_type {
|
enum udp_act_type {
|
||||||
UDP_ACT_TAP,
|
UDP_ACT_TAP,
|
||||||
|
@ -175,7 +175,7 @@ enum udp_act_type {
|
||||||
};
|
};
|
||||||
|
|
||||||
/* Activity-based aging for bindings */
|
/* 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 */
|
/* Static buffers */
|
||||||
|
|
||||||
|
@ -271,7 +271,7 @@ static void udp_invert_portmap(struct udp_port_fwd *fwd)
|
||||||
in_port_t delta = fwd->f.delta[i];
|
in_port_t delta = fwd->f.delta[i];
|
||||||
|
|
||||||
if (delta)
|
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))
|
if (ns_enter(c))
|
||||||
return 0;
|
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))
|
if (!bitmap_isset(c->udp.fwd_out.f.map, dst))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
|
|
2
udp.h
2
udp.h
|
@ -50,7 +50,7 @@ union udp_epoll_ref {
|
||||||
*/
|
*/
|
||||||
struct udp_port_fwd {
|
struct udp_port_fwd {
|
||||||
struct port_fwd f;
|
struct port_fwd f;
|
||||||
in_port_t rdelta[USHRT_MAX];
|
in_port_t rdelta[NUM_PORTS];
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
Loading…
Reference in a new issue