From 422246eb265bc0cfebb5c872ba3d4289798f9cd3 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sat, 7 Jan 2012 23:54:13 +0100 Subject: [PATCH 136/139] 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. --- doc/configuration.txt | 10 ++++++++-- src/proto_http.c | 11 +++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 3e777fa..28e7330 100644 --- 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 462126a..034a752 100644 --- 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; -- 1.7.2.3