BUG/MEDIUM: http_act: fix normalize-uri normalizer selection with a condition

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 <ykglory0216@gmail.com>

diff --git a/src/http_act.c b/src/http_act.c
index 02e1bdbe12d68e1720e63877b541bc611bbfb0bb..fddd74dfe7d8c3131aa1ecc3fe7c5dc93302ea08 100644 (file)
--- 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;
                }