Hi,
spotted while looking at
https://raw.githubusercontent.com/icing/mod_h2/master/sandbox/httpd/patches/core-protocols.patch
which include it.
CJ
Le 15/07/2014 14:27, jor...@apache.org a écrit :
Author: jorton
Date: Tue Jul 15 12:27:00 2014
New Revision: 1610674
URL: http://svn.apache.org/r1610674
Log:
SECURITY (CVE-2014-0117): Fix a crash in mod_proxy. In a reverse
proxy configuration, a remote attacker could send a carefully crafted
request which could crash a server process, resulting in denial of
service.
Thanks to Marek Kroemeke working with HP's Zero Day Initiative for
reporting this issue.
* server/util.c (ap_parse_token_list_strict): New function.
* modules/proxy/proxy_util.c (find_conn_headers): Use it here.
* modules/proxy/mod_proxy_http.c (ap_proxy_http_process_response):
Send a 400 for a malformed Connection header.
Submitted by: Edward Lu, breser, covener
Modified:
httpd/httpd/trunk/include/ap_mmn.h
httpd/httpd/trunk/include/httpd.h
httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
httpd/httpd/trunk/modules/proxy/proxy_util.c
httpd/httpd/trunk/server/util.c
Modified: httpd/httpd/trunk/server/util.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1610674&r1=1610673&r2=1610674&view=diff
==============================================================================
--- httpd/httpd/trunk/server/util.c (original)
+++ httpd/httpd/trunk/server/util.c Tue Jul 15 12:27:00 2014
@@ -1452,6 +1452,95 @@ AP_DECLARE(int) ap_find_etag_weak(apr_po
return find_list_item(p, line, tok, AP_ETAG_WEAK);
}
+/* Grab a list of tokens of the format 1#token (from RFC7230) */
+AP_DECLARE(const char *) ap_parse_token_list_strict(apr_pool_t *p,
+ const char *str_in,
+ apr_array_header_t **tokens,
+ int skip_invalid)
+{
+ int in_leading_space = 1;
+ int in_trailing_space = 0;
+ int string_end = 0;
+ const char *tok_begin;
+ const char *cur;
+
+ if (!str_in) {
+ return NULL;
+ }
+
+ tok_begin = cur = str_in;
+
+ while (!string_end) {
+ const unsigned char c = (unsigned char)*cur;
+
+ if (!TEST_CHAR(c, T_HTTP_TOKEN_STOP) && c != '\0') {
+ /* Non-separator character; we are finished with leading
+ * whitespace. We must never have encountered any trailing
+ * whitespace before the delimiter (comma) */
+ in_leading_space = 0;
+ if (in_trailing_space) {
+ return "Encountered illegal whitespace in token";
+ }
+ }
+ else if (c == ' ' || c == '\t') {
+ /* "Linear whitespace" only includes ASCII CRLF, space, and tab;
+ * we can't get a CRLF since headers are split on them already,
+ * so only look for a space or a tab */
+ if (in_leading_space) {
+ /* We're still in leading whitespace */
+ ++tok_begin;
+ }
+ else {
+ /* We must be in trailing whitespace */
+ ++in_trailing_space;
+ }
+ }
+ else if (c == ',' || c == '\0') {
+ if (!in_leading_space) {
+ /* If we're out of the leading space, we know we've read some
+ * characters of a token */
+ if (*tokens == NULL) {
+ *tokens = apr_array_make(p, 4, sizeof(char *));
+ }
+ APR_ARRAY_PUSH(*tokens, char *) =
+ apr_pstrmemdup((*tokens)->pool, tok_begin,
+ (cur - tok_begin) - in_trailing_space);
+ }
+ /* We're allowed to have null elements, just don't add them to the
+ * array */
+
+ tok_begin = cur + 1;
+ in_leading_space = 1;
+ in_trailing_space = 0;
+ string_end = (c == '\0');
+ }
+ else {
+ /* Encountered illegal separator char */
+ if (skip_invalid) {
+ /* Skip to the next separator */
+ const char *temp;
+ temp = ap_strchr_c(cur, ',');
+ if(!temp) {
+ temp = ap_strchr_c(cur, '\0');
+ }
+
+ /* Act like we haven't seen a token so we reset */
+ cur = temp - 1;
If i understand correctly, if we find an invalid char and
'skip_invalid', we first look for the next comma and start searching for
new token from there.
If no comma is found before the trailing NULL, then nothing more can be
found and we end the processing.
So shouldn't this be:
cur = temp + 1;
(i.e. go to the character *following* the comma)
Not tested, but I think that if the invalid character is the one just
before the comma, then we loop forever.
Also if the:
temp = ap_strchr_c(cur, '\0');
above is supposed to reach the end of the string and stop processing, we
have the same problem and can loop forever if the last non-NULL char is
the invalid char.
In this case, adding:
string_end = (c == '\0');
as in the normal case, would be fine.
+ in_leading_space = 1;
+ in_trailing_space = 0;
+ }
+ else {
+ return apr_psprintf(p, "Encountered illegal separator "
+ "'\\x%.2x'", (unsigned int)c);
+ }
+ }
+
+ ++cur;
+ }
+
+ return NULL;
+}
+
/* Retrieve a token, spacing over it and returning a pointer to
* the first non-white byte afterwards. Note that these tokens
* are delimited by semis and commas; and can also be delimited
