From: Willy Tarreau Date: Mon, 27 Jul 2026 08:01:05 +0000 (+0200) Subject: BUG/MEDIUM: http-fetch: reject a negative capture id in capture.{req,res}.hdr X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=ce0b1d26eab9821379c710a00ec36cb4e9c748d1;p=thirdparty%2Fhaproxy.git 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 , 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 3f3cd09ba..2441a2969 100644 --- 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 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*