MEDIUM: http: block non-ASCII characters in URIs by default

These ones are invalid and blocked unless "option accept-invalid-http-request"
is specified in the frontend. In any case, the faulty request is logged.

Note that some of the remaining invalid chars are still not checked against,
those are the invalid ones between 32 and 127 :

    34 ('"'), 60 ('<'), 62 ('>'), 92 ('\'), 94 ('^'),
    96 ('`'), 123 ('{'), 124 ('|'), 125 ('}')

Using a lookup table might be better at some point.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 3e777fad21955d05efa11e9447e2be07b6bf0581..28e733054f8c5a07abc9ec704f33aedf52c7c364 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2873,7 +2873,12 @@ no option accept-invalid-http-request
   server will emit invalid header names for whatever reason (configuration,
   implementation) and the issue will not be immediately fixed. In such a case,
   it is possible to relax HAProxy's header name parser to accept any character
-  even if that does not make sense, by specifying this option.
+  even if that does not make sense, by specifying this option. Similarly, the
+  list of characters allowed to appear in a URI is well defined by RFC3986, and
+  chars 0-31, 32 (space), 34 ('"'), 60 ('<'), 62 ('>'), 92 ('\'), 94 ('^'), 96
+  ('`'), 123 ('{'), 124 ('|'), 125 ('}'), 127 (delete) and anything above are
+  not allowed at all. Haproxy always blocks a number of them (0..32, 127). The
+  remaining ones are blocked by default unless this option is enabled.
 
   This option should never be enabled by default as it hides application bugs
   and open security breaches. It should only be deployed after a problem has
@@ -2881,7 +2886,8 @@ no option accept-invalid-http-request
 
   When this option is enabled, erroneous header names will still be accepted in
   requests, but the complete request will be captured in order to permit later
-  analysis using the "show errors" request on the UNIX stats socket. Doing this
+  analysis using the "show errors" request on the UNIX stats socket. Similarly,
+  requests containing invalid chars in the URI part will be logged. Doing this
   also helps confirming that the issue has been solved.
 
   If this option has been enabled in a "defaults" section, it can be disabled

diff --git a/src/proto_http.c b/src/proto_http.c
index 462126adbbff2b2deebaf76580c23af8825d1b3c..034a752c90d9542f49b6db6ecc62a476be1b00aa 100644 (file)
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1458,9 +1458,14 @@ const char *http_parse_reqline(struct http_msg *msg, const char *msg_buf,
                }
 
                if (likely((unsigned char)*ptr >= 128)) {
-                       /* FIXME: we should control whether we want to allow them, but
-                        * until now they were allowed.
+                       /* non-ASCII chars are forbidden unless option
+                        * accept-invalid-http-request is enabled in the frontend.
+                        * In any case, we capture the faulty char.
                         */
+                       if (msg->err_pos < -1)
+                               goto invalid_char;
+                       if (msg->err_pos == -1)
+                               msg->err_pos = ptr - msg_buf;
                        EAT_AND_JUMP_OR_RETURN(http_msg_rquri, HTTP_MSG_RQURI);
                }
 
@@ -1470,6 +1475,8 @@ const char *http_parse_reqline(struct http_msg *msg, const char *msg_buf,
                }
 
                /* OK forbidden chars, 0..31 or 127 */
+       invalid_char:
+               msg->err_pos = ptr - msg_buf;
                state = HTTP_MSG_ERROR;
                break;