From: Amos Jeffries <squid3@treenet.co.nz>
Date: Tue, 2 Aug 2016 15:06:37 +0000 (+1200)
Subject: Bug 4551: fix exceptions in new chunked decoder
X-Git-Tag: SQUID_4_0_13~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=188ad27fe52523a00e6c888aafd0bde83268bfeb;p=thirdparty%2Fsquid.git

Bug 4551: fix exceptions in new chunked decoder

... fixing incremental CRLF parsing in both chunked encoding and
status-line parsing contexts.
---

diff --git a/src/http/one/Parser.cc b/src/http/one/Parser.cc
index 1eb1ea46b8..625f788545 100644
--- a/src/http/one/Parser.cc
+++ b/src/http/one/Parser.cc
@@ -64,7 +64,11 @@ Http::One::Parser::skipLineTerminator(Http1::Tokenizer &tok) const
     if (Config.onoff.relaxed_header_parser && tok.skipOne(CharacterSet::LF))
         return true;
 
-    return false;
+    if (tok.atEnd() || (tok.remaining().length() == 1 && tok.remaining().at(0) == '\r'))
+        return false; // need more data
+
+    throw TexcHere("garbage instead of CRLF line terminator");
+    return false; // unreachable, but make naive compilers happy
 }
 
 /// all characters except the LF line terminator
diff --git a/src/http/one/Parser.h b/src/http/one/Parser.h
index d24c8c503f..b1e819a945 100644
--- a/src/http/one/Parser.h
+++ b/src/http/one/Parser.h
@@ -107,8 +107,14 @@ public:
     Http::StatusCode parseStatusCode;
 
 protected:
-    /// detect and skip the CRLF or (if tolerant) LF line terminator
-    /// consume from the tokenizer and return true only if found
+    /**
+     * detect and skip the CRLF or (if tolerant) LF line terminator
+     * consume from the tokenizer.
+     *
+     * throws if non-terminator is detected.
+     * \retval true only if line terminator found.
+     * \retval false incomplete or missing line terminator, need more data.
+     */
     bool skipLineTerminator(Http1::Tokenizer &tok) const;
 
     /// the characters which are to be considered valid whitespace
diff --git a/src/http/one/ResponseParser.cc b/src/http/one/ResponseParser.cc
index 7cdbf493cd..007cfaf72c 100644
--- a/src/http/one/ResponseParser.cc
+++ b/src/http/one/ResponseParser.cc
@@ -75,9 +75,6 @@ Http::One::ResponseParser::parseResponseStatusAndReason(Http1::Tokenizer &tok, c
         // NOTE: any whitespace after the single SP is part of the reason phrase.
     }
 
-    if (tok.atEnd())
-        return 0; // need more to be sure we have it all
-
     /* RFC 7230 says we SHOULD ignore the reason phrase content
      * but it has a definite valid vs invalid character set.
      * We interpret the SHOULD as ignoring absence and syntax, but
@@ -89,17 +86,18 @@ Http::One::ResponseParser::parseResponseStatusAndReason(Http1::Tokenizer &tok, c
     // if we got here we are still looking for reason-phrase bytes
     static const CharacterSet phraseChars = CharacterSet::WSP + CharacterSet::VCHAR + CharacterSet::OBSTEXT;
     (void)tok.prefix(reasonPhrase_, phraseChars); // optional, no error if missing
-    if (skipLineTerminator(tok)) {
-        debugs(74, DBG_DATA, "parse remaining buf={length=" << tok.remaining().length() << ", data='" << tok.remaining() << "'}");
-        buf_ = tok.remaining(); // resume checkpoint
-        return 1;
-    }
-    reasonPhrase_.clear();
-
-    if (tok.atEnd())
+    try {
+        if (skipLineTerminator(tok)) {
+            debugs(74, DBG_DATA, "parse remaining buf={length=" << tok.remaining().length() << ", data='" << tok.remaining() << "'}");
+            buf_ = tok.remaining(); // resume checkpoint
+            return 1;
+        }
+        reasonPhrase_.clear();
         return 0; // need more to be sure we have it all
 
-    debugs(74, 6, "invalid status-line. garbage in reason phrase.");
+    } catch (const std::exception &ex) {
+        debugs(74, 6, "invalid status-line: " << ex.what());
+    }
     return -1;
 }
 
diff --git a/src/http/one/TeChunkedParser.cc b/src/http/one/TeChunkedParser.cc
index a247097689..1bcaee5006 100644
--- a/src/http/one/TeChunkedParser.cc
+++ b/src/http/one/TeChunkedParser.cc
@@ -153,9 +153,6 @@ Http::One::TeChunkedParser::parseChunkExtension(Http1::Tokenizer &tok, bool skip
             buf_ = tok.remaining(); // parse checkpoint (unless there might be more token name)
     }
 
-    if (tok.atEnd())
-        return false;
-
     if (skipLineTerminator(tok)) {
         buf_ = tok.remaining(); // checkpoint
         // non-0 chunk means data, 0-size means optional Trailer follows
@@ -163,7 +160,6 @@ Http::One::TeChunkedParser::parseChunkExtension(Http1::Tokenizer &tok, bool skip
         return true;
     }
 
-    throw TexcHere("corrupted chunk extension value");
     return false;
 }
 
@@ -202,9 +198,6 @@ Http::One::TeChunkedParser::parseChunkEnd(Http1::Tokenizer &tok)
         theChunkSize = 0; // done with the current chunk
         parsingStage_ = Http1::HTTP_PARSE_CHUNK_SZ;
         return true;
-
-    } else if (!tok.atEnd()) {
-        throw TexcHere("found data between chunk end and CRLF");
     }
 
     return false;