From 30cde2c1626c8decfa58f40e5f38f447d9f6e676 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 25 Mar 2021 14:12:29 +0100 Subject: [PATCH] BUG/MINOR: http_fetch: make hdr_ip() reject trailing characters The hdr_ip() sample fetch function will try to extract IP addresses from a header field. These IP addresses are parsed using url2ipv4() and if it fails it will fall back to inet_pton(AF_INET6), otherwise will fail. There is a small problem there which is that if a field starts with an IP address and is immediately followed by some garbage, the IP address part is still returned. This is a problem with fields such as x-forwarded-for because it prevents detection of accidental corruption or bug along the chain. For example, the following string: x-forwarded-for: 1.2.3.4; 5.6.7.8 or this one: x-forwarded-for: 1.2.3.4O ( the last one being the letter 'O') would still return "1.2.3.4" despite the trailing characters. This is bad because it will silently cover broken code running on intermediary proxies and may even in some cases allow haproxy to pass improperly formatted headers after they were apparently validated, for example, if someone extracts the address from this field to place it into another one. This issue would only affect the IPv4 parser, because the IPv6 parser already uses inet_pton() which fails at the first invalid character and rejects trailing port numbers. In strict compliance with RFC7239, let's make sure that if there are any characters left in the string, the parsing fails and makes hdr_ip() return nothing. However, a special case has to be handled to support IPv4 addresses followed by a colon and a valid port number, because till now the parser used to implicitly accept them and it appears that this practice, though rare, does exist at least in Azure: https://docs.microsoft.com/en-us/azure/application-gateway/how-application-gateway-works This issue has always been there so the fix may be backported to all versions. It will need the following commit in order to work as expected: MINOR: tools: make url2ipv4 return the exact number of bytes parsed Many thanks to https://twitter.com/melardev and the BitMEX Security Team for their detailed report. (cherry picked from commit 7b0e00d943feaa6320522c376fbbff18965b1345) Signed-off-by: Willy Tarreau (cherry picked from commit 7e2e49ede43d9d65304edaae8861477ce53faab2) Signed-off-by: Willy Tarreau (cherry picked from commit 7a7b8393f4b0e6ca7a4ad9fe250370340cb8bff0) [wt: small adjustments to the doc] Signed-off-by: Willy Tarreau --- doc/configuration.txt | 6 +++++- src/http_fetch.c | 17 ++++++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 326aaca..af8fa5f 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -16074,7 +16074,11 @@ hdr_ip([[,]]) : ip (deprecated) This extracts the last occurrence of header in an HTTP request, converts it to an IPv4 or IPv6 address and returns this address. When used with ACLs, all occurrences are checked, and if is omitted, every value - of every header is checked. Optionally, a specific occurrence might be + of every header is checked. The parser strictly adheres to the format + described in RFC7239, with the extension that IPv4 addresses may optionally + be followed by a colon (':') and a valid decimal port number (0 to 65535), + which will be silently dropped. All other forms will not match and will + cause the address to be ignored. Optionally, a specific occurrence might be specified as a position number. Positive values indicate a position from the first occurrence, with 1 being the first one. Negative values indicate positions relative to the last one, with -1 being the last one. A typical use diff --git a/src/http_fetch.c b/src/http_fetch.c index 22e6abd..e07bab4 100644 --- a/src/http_fetch.c +++ b/src/http_fetch.c @@ -959,19 +959,30 @@ static int smp_fetch_hdr_val(const struct arg *args, struct sample *smp, const c /* Fetch an HTTP header's IP value. takes a mandatory argument of type string * and an optional one of type int to designate a specific occurrence. - * It returns an IPv4 or IPv6 address. + * It returns an IPv4 or IPv6 address. Addresses surrounded by invalid chars + * are rejected. However IPv4 addresses may be followed with a colon and a + * valid port number. */ static int smp_fetch_hdr_ip(const struct arg *args, struct sample *smp, const char *kw, void *private) { - int ret; struct buffer *temp = get_trash_chunk(); + int ret, len; + int port; while ((ret = smp_fetch_hdr(args, smp, kw, private)) > 0) { if (smp->data.u.str.data < temp->size - 1) { memcpy(temp->area, smp->data.u.str.area, smp->data.u.str.data); temp->area[smp->data.u.str.data] = '\0'; - if (url2ipv4((char *) temp->area, &smp->data.u.ipv4)) { + len = url2ipv4((char *) temp->area, &smp->data.u.ipv4); + if (len == smp->data.u.str.data) { + /* plain IPv4 address */ + smp->data.type = SMP_T_IPV4; + break; + } else if (len > 0 && temp->area[len] == ':' && + strl2irc(temp->area + len + 1, smp->data.u.str.data - len - 1, &port) == 0 && + port >= 0 && port <= 65535) { + /* IPv4 address suffixed with ':' followed by a valid port number */ smp->data.type = SMP_T_IPV4; break; } else if (inet_pton(AF_INET6, temp->area, &smp->data.u.ipv6)) { -- 1.7.10.4