From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Thu, 25 Aug 2016 17:26:02 +0000 (-0600)
Subject: MUST respond with 414 (URI Too Long) when request target exceeds limits.
X-Git-Tag: SQUID_4_0_14~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f8b58a682a7d4eb41d07e108409de3c752f42e44;p=thirdparty%2Fsquid.git

MUST respond with 414 (URI Too Long) when request target exceeds limits.

Before the fix, Squid simply closed client connection after receiving a
huge URI (or a huge request-line), violating the RFC 7230 MUST. This
happened because a high-level Must(have buffer space) check in
ConnStateData::clientParseRequests() would throw an exception. Now these
problems are detected inside the low-level RequestParser code, where we
can distinguish huge URIs from huge methods.
---

diff --git a/src/http/one/RequestParser.cc b/src/http/one/RequestParser.cc
index a1c18942e2..d2fb2536d5 100644
--- a/src/http/one/RequestParser.cc
+++ b/src/http/one/RequestParser.cc
@@ -85,6 +85,10 @@ Http::One::RequestParser::parseMethodField(Http1::Tokenizer &tok)
         return false;
     }
     method_ = HttpRequestMethod(methodFound);
+
+    if (!skipDelimiter(tok.skipAll(DelimiterCharacters()), "after method"))
+        return false;
+
     return true;
 }
 
@@ -212,17 +216,17 @@ Http::One::RequestParser::parseHttpVersionField(Http1::Tokenizer &tok)
  * we just check how many character the caller has skipped.
  */
 bool
-Http::One::RequestParser::skipDelimiter(const size_t count)
+Http::One::RequestParser::skipDelimiter(const size_t count, const char *where)
 {
     if (count <= 0) {
-        debugs(33, ErrorLevel(), "invalid request-line: missing delimiter");
+        debugs(33, ErrorLevel(), "invalid request-line: missing delimiter " << where);
         parseStatusCode = Http::scBadRequest;
         return false;
     }
 
     // tolerant parser allows multiple whitespace characters between request-line fields
     if (count > 1 && !Config.onoff.relaxed_header_parser) {
-        debugs(33, ErrorLevel(), "invalid request-line: too many delimiters");
+        debugs(33, ErrorLevel(), "invalid request-line: too many delimiters " << where);
         parseStatusCode = Http::scBadRequest;
         return false;
     }
@@ -270,19 +274,28 @@ Http::One::RequestParser::parseRequestFirstLine()
     static const CharacterSet lineChars = CharacterSet::LF.complement("notLF");
     ::Parser::Tokenizer lineTok(buf_);
     if (!lineTok.prefix(line, lineChars) || !lineTok.skip('\n')) {
+        if (buf_.length() >= Config.maxRequestHeaderSize) {
+            /* who should we blame for our failure to parse this line? */
+
+            Http1::Tokenizer methodTok(buf_);
+            if (!parseMethodField(methodTok))
+                return -1; // blame a bad method (or its delimiter)
+
+            // assume it is the URI
+            debugs(74, ErrorLevel(), "invalid request-line: URI exceeds " <<
+                    Config.maxRequestHeaderSize << "-byte limit");
+            parseStatusCode = Http::scUriTooLong;
+            return -1;
+        }
         debugs(74, 5, "Parser needs more data");
         return 0;
     }
 
     Http1::Tokenizer tok(line);
-    const CharacterSet &delimiters = DelimiterCharacters();
 
     if (!parseMethodField(tok))
         return -1;
 
-    if (!skipDelimiter(tok.skipAll(delimiters)))
-        return -1;
-
     /* now parse backwards, to leave just the URI */
     if (!skipTrailingCrs(tok))
         return -1;
@@ -290,7 +303,7 @@ Http::One::RequestParser::parseRequestFirstLine()
     if (!parseHttpVersionField(tok))
         return -1;
 
-    if (!http0() && !skipDelimiter(tok.skipAllTrailing(delimiters)))
+    if (!http0() && !skipDelimiter(tok.skipAllTrailing(DelimiterCharacters()), "before protocol version"))
         return -1;
 
     /* parsed everything before and after the URI */
diff --git a/src/http/one/RequestParser.h b/src/http/one/RequestParser.h
index 107b8cf4f7..0d2271d890 100644
--- a/src/http/one/RequestParser.h
+++ b/src/http/one/RequestParser.h
@@ -52,7 +52,7 @@ private:
     bool parseMethodField(Http1::Tokenizer &);
     bool parseUriField(Http1::Tokenizer &);
     bool parseHttpVersionField(Http1::Tokenizer &);
-    bool skipDelimiter(const size_t count);
+    bool skipDelimiter(const size_t count, const char *where);
     bool skipTrailingCrs(Http1::Tokenizer &tok);
 
     bool http0() const {return !msgProtocol_.major;}