BUG/MINOR: server: fix off-by-one error when parsing and copying source port range

When allocating a source port range, port_range_alloc_range() allocates
a ring structure of size n + 1 to accommodate a sentinel slot for the
lock-free ring buffer.

However, both srv_parse_source() and srv_conn_src_sport_range_cpy()
were incorrectly using range->size directly, filling and copying the
sentinel slot as if it were a valid port. This ->size port_range struct
field should never be used.

This off-by-one error caused an extra port to be populated. When copying a
configuration (e.g. via default-server or server-template), this extra
slot became allocatable, allowing connections to bind beyond the configured
port range (e.g. binding port 5002 when 5000-5001 was set). Furthermore, if
the range reached port 65535, the extra port wrapped to 0, producing
CO_ER_PORT_RANGE connection failures.

Fix this by introducing port_range_count() to cleanly return the number of
usable ports (range->size - 1), and use it in both srv_parse_source()
and srv_conn_src_sport_range_cpy().

Many thanks to Red Hat and AISLE Research for reporting this.

Must be backported as far as 2.6.

diff --git a/include/haproxy/port_range.h b/include/haproxy/port_range.h
index 768b711c61338bf75f2be7cba9b71ad292ddd4e1..173ebed85aa4c239edeb58d7faaa087cd2f312c0 100644 (file)
--- a/include/haproxy/port_range.h
+++ b/include/haproxy/port_range.h
@@ -92,6 +92,14 @@ static inline void port_range_release_port(struct port_range *range, int port)
        port_range_release(range);
 }
 
+/* Return the number of usable ports in the range <range>.
+ * Note that range->size includes an extra slot used as a ring buffer sentinel.
+ */
+static inline int port_range_count(struct port_range *range)
+{
+       return range ? range->size - 1 : 0;
+}
+
 /* return a new initialized port range of N ports. The ports are not
  * filled in, it's up to the caller to do it.
  */

diff --git a/src/server.c b/src/server.c
index 1090b2307ff979aa5b239a1334fe2826d8abeac3..d38e9d1edba4c04f42cb265e5cbdea00c50de2b9 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -1876,7 +1876,8 @@ static int srv_parse_source(char **args, int *cur_arg,
                        ha_alert("Server '%s': Out of memory (sport_range)\n", args[0]);
                        goto err;
                }
-               for (i = 0; i < newsrv->conn_src.sport_range->size; i++)
+
+               for (i = 0; i < port_range_count(newsrv->conn_src.sport_range); i++)
                        newsrv->conn_src.sport_range->ports[i] = port_low + i;
        }
 
@@ -2709,7 +2710,7 @@ static void srv_conn_src_sport_range_cpy(struct server *srv, const struct server
 {
        int range_sz;
 
-       range_sz = src->conn_src.sport_range->size;
+       range_sz = port_range_count(src->conn_src.sport_range);
        if (range_sz > 0) {
                srv->conn_src.sport_range = port_range_alloc_range(range_sz);
                if (srv->conn_src.sport_range != NULL) {