BUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param

If a regex to match the PATH_INFO parameter is configured, it systematically
fails if a newline or a null character is present in the URL-decoded path. So,
from the moment there is at least a "%0a" or a "%00" in the request path, we
always fail to get the PATH_INFO parameter and all the decoded path is used for
the SCRIPT_NAME parameter.

It is probably not the expected behavior. Because, most of time, these
characters are not expected at all in a path, an error is now triggered when one
of these characters is found in the URL-decoded path before trying to execute
the path_info regex. However, this test is not performed if there is no regex
configured.

Note that in reality, the newline character is only a problem when HAProxy is
complied with pcre or pcre2 library and conversely, the null character is only a
problem for the libc's regex library. But both are always excluded to avoid any
inconsistency depending on compile options.

An alternative, not implemented yet, is to replace these characters by another
one. If someone complains about this behavior, it will be re-evaluated.

This patch must be backported to all versions supporting the FastCGI
applications, so to 2.1 for now.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index e39db29b4d80fd53a5421dceadd2bac6fdec51e2..f2124db95edb14b1c01d056452ef616cf03ac559 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -18726,11 +18726,18 @@ pass-header <name> [ { if | unless } <condition> ]
   the FastCGI application because they are already converted into parameters.
 
 path-info <regex>
-  Define a regular expression to extract the script-name and the path-info
-  from the URI. Thus, <regex> should have two captures: the first one to
-  capture the script name and the second one to capture the path-info. It is an
-  optional setting. If it is not defined, no matching is performed on the
-  URI. and the FastCGI parameters PATH_INFO and PATH_TRANSLATED are not filled.
+  Define a regular expression to extract the script-name and the path-info from
+  the URL-decoded path. Thus, <regex> should have two captures: the first one
+  to capture the script name and the second one to capture the path-info. It is
+  an optional setting. If it is not defined, no matching is performed on the
+  path. and the FastCGI parameters PATH_INFO and PATH_TRANSLATED are not filled.
+
+  For security reason, when this regular expression is defined, the newline and
+  the null characters are forbiden from the path, once URL-decoded. The reason
+  to such limitation is because otherwise the matching always fails (due to a
+  limitation one the way regular expression are executed in HAProxy). So if one
+  of these two characters is found in the URL-decoded path, an error is
+  returned to the client. The principle of least astonishment is applied here.
 
   Example :
      path-info ^(/.+\.php)(/.*)?$

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 38c2bfd64d772f3651886642fb858f415fcdeb77..12d29d6863ae0289e2676eea02a9201c96c153a0 100644 (file)
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -1345,6 +1345,15 @@ static int fcgi_set_default_param(struct fcgi_conn *fconn, struct fcgi_strm *fst
                if (!fconn->app->pathinfo_re)
                        goto check_index;
 
+               /* If some special characters are found in the decoded path (\n
+                * or \0), the PATH_INFO regex cannot match. This is theorically
+                * valid, but probably unexpected, to have such characters. So,
+                * to avoid any suprises, an error is triggered in this
+                * case.
+                */
+               if (istchr(path, '\n') || istchr(path, '\0'))
+                       goto error;
+
                /* The regex does not match, just to the last part and see if
                 * the index must be used.
                 */