BUG/MEDIUM: http-fetch: reject a negative capture id in capture.{req,res}.hdr

The "capture.req.hdr" and "capture.res.hdr" sample fetches use their integer
argument directly as an index in the stream's captures array:

	idx = args->data.sint;

	if (idx > (fe->nb_req_cap - 1) || smp->strm->req_cap == NULL ||
	    smp->strm->req_cap[idx] == NULL)
		return 0;

Only the upper bound is verified, and unlike "req.hdr" and friends, which rely
on val_hdr() to enforce the lower bound of the occurrence number, these two
keywords are declared with no argument checker at all. A negative identifier is
therefore accepted at boot, and at runtime req_cap[-1] is read; if the pointer
found there is not NULL it is then passed to strlen() and returned as a string.

This is trivially reproduced with a frontend containing:

	capture request header Host len 32
	http-request return status 200 hdr X-Cap "%[capture.req.hdr(-1)]"

which segfaults the worker on the very first request.

Since a negative capture identifier is meaningless, the cleanest fix is to
reject it at configuration parsing time, as is done for the header occurrence.
Let's add a val_cap_id() checker and reference it from both keywords.

Note that the "capture-req"/"capture-res" converters in http_conv.c index the
same array with an unchecked value too, but they are saved by the list walk
that precedes the access and which stops on a NULL <hdr>, so they only fail to
capture. They are left untouched.

This bug has been there since the keywords were introduced, so this should be
backported to all supported versions.

diff --git a/src/http_fetch.c b/src/http_fetch.c
index 3f3cd09bae89d89df14e59495a3bef2040c32710..2441a2969b9d881b35c231e5dcff2cc8b70411c1 100644 (file)
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -2283,6 +2283,23 @@ int val_hdr(struct arg *arg, char **err_msg)
        return 1;
 }
 
+/* This function is used to validate the argument passed to the
+ * "capture.req.hdr" and "capture.res.hdr" fetch keywords. The capture
+ * identifier is used as an index in the stream's captures array, so it must not
+ * be negative. It is assumed that the type is already the correct one. Returns
+ * 0 on error, non-zero if OK. If <err_msg> is not NULL, it will be filled with a
+ * pointer to an error message in case of error, that the caller is responsible
+ * for freeing. The initial location must either be freeable or NULL.
+ */
+static int val_cap_id(struct arg *arg, char **err_msg)
+{
+       if (arg && arg[0].type == ARGT_SINT && arg[0].data.sint < 0) {
+               memprintf(err_msg, "capture identifier must be >= 0");
+               return 0;
+       }
+       return 1;
+}
+
 int val_query(struct arg *args, char **err_msg)
 {
        int val = 0;
@@ -2321,14 +2338,14 @@ static struct sample_fetch_kw_list sample_fetch_keywords = {ILH, {
        { "baseq",              smp_fetch_base,               0,                NULL,   SMP_T_STR,  SMP_USE_HRQHV },
 
        /* capture are allocated and are permanent in the stream */
-       { "capture.req.hdr",    smp_fetch_capture_req_hdr,    ARG1(1,SINT),     NULL,   SMP_T_STR,  SMP_USE_HRQHP },
+       { "capture.req.hdr",    smp_fetch_capture_req_hdr,    ARG1(1,SINT),     val_cap_id, SMP_T_STR,  SMP_USE_HRQHP },
 
        /* retrieve these captures from the HTTP logs */
        { "capture.req.method", smp_fetch_capture_req_method, 0,                NULL,   SMP_T_STR,  SMP_USE_HRQHP },
        { "capture.req.uri",    smp_fetch_capture_req_uri,    0,                NULL,   SMP_T_STR,  SMP_USE_HRQHP },
        { "capture.req.ver",    smp_fetch_capture_req_ver,    0,                NULL,   SMP_T_STR,  SMP_USE_HRQHP },
 
-       { "capture.res.hdr",    smp_fetch_capture_res_hdr,    ARG1(1,SINT),     NULL,   SMP_T_STR,  SMP_USE_HRSHP },
+       { "capture.res.hdr",    smp_fetch_capture_res_hdr,    ARG1(1,SINT),     val_cap_id, SMP_T_STR,  SMP_USE_HRSHP },
        { "capture.res.ver",    smp_fetch_capture_res_ver,    0,                NULL,   SMP_T_STR,  SMP_USE_HRQHP },
 
        /* cookie is valid in both directions (eg: for "stick ...") but cook*