Skip to content

Commit 37204a7

Browse files
haproxyFredhaproxyFred
committed
BUG/MINOR: sample: reject a \0 byte in url_dec, json_query and jwt_*_query
A string sample cannot contain a \0 byte by design: this is what distinguishes it from a binary sample, and every consumer relies on it. Yet url_dec decoded "%00" to a \0, json_query decoded "\u0000" to one, and jwt_header_query/jwt_payload_query could return one from the base64 decoded JSON. Such a sample was then matched by "-m str" on its prefix only, so "/public%00/admin" matched "/public" and an allow-list could be bypassed. Commit 442f583 ("BUG/MINOR: pattern: do not match a string with an embedded \0 on its prefix") worked around it in pat_match_str(), which is the wrong place since such a string must not exist at all. This commit reverts it and fixes the producers instead: url_decode() now fails on "%00" like it does on an invalid sequence, and json_query and jwt_*_query fail on a decoded \0. This is also cheaper, as the string is checked once where it is produced instead of on every call to pat_match_str(). This must be backported to all stable branches, in place of 442f583 where it was not yet backported.
1 parent 209c027 commit 37204a7

3 files changed

Lines changed: 13 additions & 8 deletions

File tree

‎src/pattern.c‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,13 +499,6 @@ struct pattern *pat_match_str(struct sample *smp, struct pattern_expr *expr, int
499499
struct pattern *ret = NULL;
500500
struct lru64 *lru = NULL;
501501

502-
/* Patterns never contain a NUL byte, so a sample with an embedded
503-
* NUL cannot be an exact match. Without this check, the tree lookup
504-
* would stop at the NUL and match on the prefix only.
505-
*/
506-
if (memchr(smp->data.u.str.area, 0, smp->data.u.str.data))
507-
return NULL;
508-
509502
/* Lookup a string in the expression's pattern tree. */
510503
if (!eb_is_empty(&expr->pattern_tree)) {
511504
if (!pat_match_ensure_str(smp))

‎src/sample.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4703,6 +4703,10 @@ static int sample_conv_json_query(const struct arg *args, struct sample *smp, vo
47034703
return 0;
47044704
}
47054705

4706+
/* mjson might return an embedded \0 that is not valid as a string */
4707+
if (memchr(trash->area, 0, len))
4708+
return 0;
4709+
47064710
trash->data = len;
47074711
smp->data.u.str = *trash;
47084712
smp->data.type = SMP_T_STR;
@@ -4967,6 +4971,10 @@ static int sample_conv_jwt_member_query(const struct arg *args, struct sample *s
49674971
if (ret == -1)
49684972
goto end;
49694973

4974+
/* mjson might return an embedded \0 that is not valid as a string */
4975+
if (memchr(decoded_header->area, 0, ret))
4976+
goto end;
4977+
49704978
decoded_header->data = ret;
49714979
if (args[0].type != ARGT_STR) {
49724980
smp->data.u.str = *decoded_header;

‎src/tools.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2654,7 +2654,11 @@ int url_decode(char *string, int in_form)
26542654
case '%' :
26552655
if (!ishex(in[1]) || !ishex(in[2]))
26562656
goto end;
2657-
*out++ = (hex2i(in[1]) << 4) + hex2i(in[2]);
2657+
*out = (hex2i(in[1]) << 4) + hex2i(in[2]);
2658+
/* forbid %00 which cannot be represented in a string */
2659+
if (!*out)
2660+
goto end;
2661+
out++;
26582662
in += 2;
26592663
break;
26602664
case '?':

0 commit comments

Comments
 (0)