The normalizers that accept an optional sub-option ("strict" for
percent-to-uppercase and percent-decode-unreserved, "full" for
path-strip-dotdot) only set rule->action in the branch handling the
sub-option and in the branch handling the end of the arguments. When
the sub-option is omitted and an "if" or "unless" keyword directly
follows the normalizer name, none of these branches is taken and
rule->action is left untouched.
As a result the rule ends up with action 0, which is
ACT_NORMALIZE_URI_PATH_MERGE_SLASHES, and a configuration such as:
http-request normalize-uri percent-decode-unreserved if { ... }
silently merges duplicate slashes instead of decoding percent
sequences. This is easily missed because the rule still does
something, and for a URI without duplicate slashes the request is
left unchanged, which looks like the rule was simply skipped.
Set the default action right after the normalizer name is matched and
let the sub-option overwrite it. The check rejecting unknown arguments
is kept, guarded by a test on the argument being non-empty.
This should fix GitHub issue #3493.
This may be backported to all stable versions where the affected
normalizers exist. The three of them were introduced in 2.4-dev by
commits 560e1a635 ("MINOR: uri_normalizer: Add support for supressing
leading `../` for dotdot normalizer"), a40719337 ("MINOR:
uri_normalizer: Add a `percent-upper` normalizer") and 2e4a18e04
("MINOR: uri_normalizer: Add a `percent-decode-unreserved`
normalizer").
Signed-off-by: Youngkwang Lee <[email protected]>
---
src/http_act.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/http_act.c b/src/http_act.c
index 02e1bdbe1..fddd74dfe 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -437,14 +437,14 @@ static enum act_parse_ret parse_http_normalize_uri(const
char **args, int *orig_
else if (strcmp(args[cur_arg], "path-strip-dotdot") == 0) {
cur_arg++;
+ rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT;
+
if (strcmp(args[cur_arg], "full") == 0) {
cur_arg++;
rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT_FULL;
}
- else if (!*args[cur_arg]) {
- rule->action = ACT_NORMALIZE_URI_PATH_STRIP_DOTDOT;
- }
- else if (strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
+ else if (*args[cur_arg] &&
+ strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
memprintf(err, "unknown argument '%s' for
'path-strip-dotdot' normalizer", args[cur_arg]);
return ACT_RET_PRS_ERR;
}
@@ -457,14 +457,14 @@ static enum act_parse_ret parse_http_normalize_uri(const
char **args, int *orig_
else if (strcmp(args[cur_arg], "percent-to-uppercase") == 0) {
cur_arg++;
+ rule->action = ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE;
+
if (strcmp(args[cur_arg], "strict") == 0) {
cur_arg++;
rule->action =
ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE_STRICT;
}
- else if (!*args[cur_arg]) {
- rule->action = ACT_NORMALIZE_URI_PERCENT_TO_UPPERCASE;
- }
- else if (strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
+ else if (*args[cur_arg] &&
+ strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
memprintf(err, "unknown argument '%s' for
'percent-to-uppercase' normalizer", args[cur_arg]);
return ACT_RET_PRS_ERR;
}
@@ -472,14 +472,14 @@ static enum act_parse_ret parse_http_normalize_uri(const
char **args, int *orig_
else if (strcmp(args[cur_arg], "percent-decode-unreserved") == 0) {
cur_arg++;
+ rule->action = ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED;
+
if (strcmp(args[cur_arg], "strict") == 0) {
cur_arg++;
rule->action =
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED_STRICT;
}
- else if (!*args[cur_arg]) {
- rule->action =
ACT_NORMALIZE_URI_PERCENT_DECODE_UNRESERVED;
- }
- else if (strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
+ else if (*args[cur_arg] &&
+ strcmp(args[cur_arg], "if") != 0 &&
strcmp(args[cur_arg], "unless") != 0) {
memprintf(err, "unknown argument '%s' for
'percent-decode-unreserved' normalizer", args[cur_arg]);
return ACT_RET_PRS_ERR;
}
--
2.45.1
