Ensure that a variable is used in the path when using 'rewrite ^ $request_uri;', to ensure double-encoding does not occur.

Also add testcases.

Signed-off-by: Joshua Rogers <Joshua@Joshua.Hu>

Author: Joshua Rogers

gixy/plugins/proxy_pass_normalized.py

@@ -14,7 +14,7 @@ class proxy_pass_normalized(Plugin):
 
     summary = 'Detect path after host in proxy_pass (potential URL decoding issue)'
     severity = gixy.severity.MEDIUM
-    description = ("A slash immediately after the host in proxy_pass leads to the path being decoded and normalized before proxying downstream, leading to unexpected behavior related to encoded slashes.")
+    description = ("A path (beginning with a slash) after the host in proxy_pass leads to the path being decoded and normalized before proxying downstream, leading to unexpected behavior related to encoded slashes like %2F..%2F. Likewise, the usage of 'rewrite ^ $request_uri;' without using '$1' or '$uri' (or another captured group) in the path of proxy_pass leads to double-encoding of paths.")
     help_url = 'https://joshua.hu/proxy-pass-nginx-decoding-normalizing-url-path-dangerous#nginx-proxy_pass'
     directives = ['proxy_pass']
 
@@ -24,6 +24,8 @@ def __init__(self, config):
 
     def audit(self, directive):
         proxy_pass_args = directive.args
+        rewrite_fail = False
+        num_pattern = r'\$\d+'
 
         if not proxy_pass_args:
             return
@@ -33,20 +35,34 @@ def audit(self, directive):
         if not parsed:
             return
 
-        if not parsed.group('path'):
-            return
-
-
         for rewrite in directive.find_directives_in_scope("rewrite"):
             if hasattr(rewrite, 'pattern') and hasattr(rewrite, 'replace'):
                 if rewrite.pattern == '^' and rewrite.replace == '$request_uri':
-                    return
+                    if parsed.group('path'):
+                        match = re.search(num_pattern, parsed.group('path'))
+                        if match or '$uri' in parsed.group('path'):
+                            return
+                        else:
+                            rewrite_fail = True
+                            break
+                    else:
+                        if not parsed.group('host'):
+                            return # ?!
+                        match = re.search(num_pattern, parsed.group('host'))
+                        if match or '$uri' in parsed.group('host'):
+                            return
+                        else:
+                            rewrite_fail = True
+                            break
+
+        if not parsed.group('path') and not rewrite_fail:
+            return
 
         self.add_issue(
             severity=self.severity,
             directive=[directive, directive.parent],
             reason=(
-                "Found a slash (and possibly more) after the hostname in proxy_pass, without using $request_uri."
-                "This can lead to path decoding issues."
+                "Found a path after the host in proxy_pass, without using $request_uri and a variable (such as $1 or $uri). "
+                "This can lead to path decoding issues or double-encoding issues."
             )
         )

tests/plugins/simply/proxy_pass_normalized/missing_variable.conf

@@ -0,0 +1,7 @@
+location /b {
+  rewrite ^ $request_uri; # Sets the $1/$uri variable to the raw path
+  proxy_pass http://127.0.0.1:8000/; # No $1 or $uri or other variable, resulting in path being double-encoded
+}
+
+# Request received by nginx: /%2F
+# Request received by backend: /%252F

tests/plugins/simply/proxy_pass_normalized/missing_variable_fp.conf

@@ -0,0 +1,7 @@
+location /b {
+  rewrite ^ $request_uri; # Sets the $1/$uri variable to the raw path
+  proxy_pass http://127.0.0.1:8000/$1; # $1 used, resulting in path being passed as the raw path from the original request.
+}
+
+# Request received by nginx: /%2F
+# Request received by backend: /%2F # Possibly also receives //%2F)

tests/plugins/simply/proxy_pass_normalized/missing_variable_nopath.conf

@@ -0,0 +1,7 @@
+location /b {
+  rewrite ^ $request_uri; # Sets the $1/$uri variable to the raw path
+  proxy_pass http://127.0.0.1:8000; # No $1 or $uri or other variable in either host or path, resulting in path being double-encoded
+}
+
+# Request received by nginx: /%2F
+# Request received by backend: /%252F

tests/plugins/simply/proxy_pass_normalized/missing_variable_nopath_fp.conf

@@ -0,0 +1,8 @@
+location /b {
+  rewrite ^ $request_uri; # Sets the $1/$uri variable to the raw path
+  proxy_pass http://127.0.0.1:8000$1; # $1 used in host, resulting in path being passed as the raw path from the original request.
+}
+
+# Request received by nginx: /%2F
+# Request received by backend: /%2F # Possibly also receives //%2F)
+# This is actually no different than proxy_pass_path_fp.conf since $1 is not a path.

tests/plugins/simply/proxy_pass_normalized/proxy_pass_path.conf

@@ -1,3 +1,6 @@
 location / {
-  proxy_pass http://server/;
+  proxy_pass http://server/; # Path "/" used, resulting in proxy_pass urldecoding the path.
 }
+
+# Request received by nginx: /%2F
+# Request received by backend: //

tests/plugins/simply/proxy_pass_normalized/proxy_pass_path_fp.conf

@@ -1,3 +1,6 @@
 location / {
-  proxy_pass http://downstream;
+  proxy_pass http://downstream; # No rewrite rules, and no path used: no extra/missing urldecoding/urlencoding occurs.
 }
+
+# Request received by nginx: /%2F
+# Request received by backend: /%2F

tests/plugins/simply/proxy_pass_normalized/rewrite_with_return_fp.conf

@@ -0,0 +1,9 @@
+location /1/ {
+  rewrite ^ $request_uri; # Sets $1/$uri to raw path.
+  rewrite ^/1(/.*) $1 break; # Extracts everything after /1 and places it into $1/$uri.
+  return 400; # extremely important! # If rewrite rule does not break (e.g. //1/), return 400, otherwise the location-block will fall-through and proxy_pass will not actually happen at all.
+  proxy_pass http://127.0.0.1:8080/$1; # $1 used, resulting in path being passed as the raw path from the original request.
+}
+
+# Request received by nginx: /1/%2F
+# Request received by backend: /%2F (or possibly //%2F)

tests/plugins/simply/rewrite_with_return.conf

@@ -0,0 +1,9 @@
+location /1/ {
+  rewrite ^ $request_uri; # Sets $1/$uri to raw path
+  rewrite ^/1(/.*) $1 break; # Extracts everything after /1
+  return 400; # extremely important! # If rewrite rule does not break (e.g. //1/), return 400, otherwise the location-block will fall-through.
+  proxy_pass http://127.0.0.1:8080; # No $1 or $uri or other variable, resulting in path being double-encoded
+}
+
+# Request received by nginx: /1/%2F
+# Request received by backend: /%252F (or possibly //%252F)