BUG/MINOR: cfgparse-listen: reject negative capture lengths

"capture request header", "capture response header" and "capture cookie"
read their length with atol() and never check it, unlike "declare capture"
and "http-request capture" which reject lengths <= 0. A negative length,
or one that does not fit in an int such as 4294967295, is thus accepted,
and the first matching message crashes the process:

  - for headers, the capture pool is created with size len+1 (0 for -1),
    then http_capture_headers() calls isttrim(v, h->len), which converts
    -1 to SIZE_MAX and does not trim anything, so the whole header value
    is copied into the tiny pool object. ASan reports a heap-buffer-overflow
    WRITE from http_capture_headers(), and a release build usually dies
    soon after with a pool tag mismatch, a malloc() corruption abort or a
    SIGSEGV;

  - for the cookie, http_manage_client_side_cookies() clamps log_len to
    capture_len (-1) and calls memcpy() with (size_t)-1 (ASan:
    negative-size-param).

Let's reject negative lengths and lengths that do not fit in an int at
parsing time. A length of zero is still accepted since it is harmless
(it captures an empty string) and some configurations may rely on it. A
reg-test is added.

This should be backported to all stable branches. The parser was moved
to proxy_parse_capture() in 3.5-dev, older branches have the same atol()
calls in cfg_parse_listen().

diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index a7207a7645894917a67bc3a8a15f7d682dea169c..0d3284b0b01305086c8d6853ddd221d1e02e59cc 100644 (file)
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -1189,6 +1189,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
                                char **err)
 {
        struct cap_hdr *hdr;
+       long len;
 
        warnifnotcap(curpx, PR_CAP_FE, file, line, args[0], NULL);
 
@@ -1208,6 +1209,12 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
                        return -1;
                }
 
+               len = atol(args[4]);
+               if (len < 0 || len > INT_MAX) {
+                       memprintf(err, "'%s %s': invalid length '%s'.", args[0], args[1], args[4]);
+                       return -1;
+               }
+
                name = strdup(args[2]);
                if (!name)
                        goto alloc_error;
@@ -1215,7 +1222,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
                free(curpx->capture_name);
                curpx->capture_name = name;
                curpx->capture_namelen = strlen(name);
-               curpx->capture_len = atol(args[4]);
+               curpx->capture_len = len;
                curpx->to_log |= LW_COOKIE;
                return 0;
        }
@@ -1231,6 +1238,12 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
                return -1;
        }
 
+       len = atol(args[5]);
+       if (len < 0 || len >= INT_MAX) {
+               memprintf(err, "'%s %s %s': invalid length '%s'.", args[0], args[1], args[2], args[5]);
+               return -1;
+       }
+
        hdr = calloc(1, sizeof(*hdr));
        if (!hdr)
                goto alloc_error;
@@ -1240,7 +1253,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
                goto alloc_err_free_hdr;
 
        hdr->namelen = strlen(args[3]);
-       hdr->len = atol(args[5]);
+       hdr->len = len;
        hdr->pool = create_pool("caphdr", hdr->len + 1, MEM_F_SHARED);
        if (!hdr->pool)
                goto alloc_err_free_name;