BUG/MINOR: http: fix an out-of-bounds read in http_get_host_port() on empty host

http_get_host_port() walks backwards from the end of the host looking for the
first non-digit, then checks whether it is a colon:

	start = istptr(host);
	end = istend(host);
	for (ptr = end; ptr > start && isdigit((unsigned char)*--ptr););

	/* no port found */
	if (likely(*ptr != ':'))
		return IST_NULL;

When <host> is empty, the loop condition fails immediately, the pre-decrement
is never evaluated and <ptr> is left equal to <end>, so *ptr reads one byte
past the end of the string. With an IST_NULL argument this is a NULL
dereference.

Both cases are reachable with an empty host: h1_validate_mismatch_authority()
calls it on the Host header value, which may be empty ("Host:\r\n") while an
absolute-form request URI is used, and http_scheme_based_normalize() calls it
on the authority extracted from the URI, which is empty for a request like
"GET http:///x HTTP/1.1". In practice the extra byte always lies inside the
request buffer, so the observable effect is limited to possibly mistaking a
neighbour byte for a colon and returning a bogus port, but it remains an
out-of-bounds read and the helper must be usable with an unset ist.

Let's return IST_NULL right away for an empty host.

This was introduced by commit 658f97162 ("MINOR: http: Add function to get port
part of a host") in 2.7-dev2, so it should be backported to 2.8 and above.

diff --git a/src/http.c b/src/http.c
index 421fe9b23ca6efc9da04ee2dbe1cdedefe4acaf0..e7e78300da978948c8b17e21406202b72d6d06c9 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -539,9 +539,9 @@ void http_status_del_range(long *array, uint low, uint high)
 }
 
 /* Returns the ist string corresponding to port part (without ':') in the host
- * <host>, IST_NULL if no ':' is found or an empty IST if there is no digit. In
- * the last case, the result is the original ist trimmed to 0. So be sure to test
- * the result length before doing any pointer arithmetic.
+ * <host>, IST_NULL if <host> is empty or if no ':' is found, or an empty IST if
+ * there is no digit. In the last case, the result is the original ist trimmed to
+ * 0. So be sure to test the result length before doing any pointer arithmetic.
 */
 struct ist http_get_host_port(const struct ist host)
 {
@@ -549,6 +549,13 @@ struct ist http_get_host_port(const struct ist host)
 
        start = istptr(host);
        end = istend(host);
+       if (start == end) {
+               /* empty (or unset) host, nothing to look for. This must be
+                * tested first, otherwise the loop below would leave <ptr> on
+                * <end> and the test on ':' would read out of the string.
+                */
+               return IST_NULL;
+       }
        for (ptr = end; ptr > start && isdigit((unsigned char)*--ptr););
 
        /* no port found */