From: Amos Jeffries <yadij@users.noreply.github.com>
Date: Fri, 14 Aug 2020 15:05:31 +0000 (+0000)
Subject: Forbid obs-fold and bare CR whitespace in framing header fields (#701)
X-Git-Tag: SQUID_4_13~3
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=e785673943961a90760ad71b36a40478bca2398d;p=thirdparty%2Fsquid.git

Forbid obs-fold and bare CR whitespace in framing header fields (#701)

Header folding has been used for various attacks on HTTP proxies and
servers. RFC 7230 prohibits sending obs-fold (in any header field) and
allows the recipient to reject messages with folded headers. To reduce
the attack space, Squid now rejects messages with folded Content-Length
and Transfer-Encoding header field values. TODO: Follow RFC 7230 status
code recommendations when rejecting.

Bare CR is a CR character that is not followed by a LF character.
Similar to folding, bare CRs has been used for various attacks. HTTP
does not allow sending bare CRs in Content-Length and Transfer-Encoding
header field values. Squid now rejects messages with bare CR characters
in those framing-related field values.

When rejecting, Squid informs the admin with a level-1 WARNING such as

    obs-fold seen in framing-sensitive Content-Length: ...
---

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 9e5e47fb34..80c23458eb 100644
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -317,6 +317,9 @@ HttpHeader::update(HttpHeader const *fresh)
     return true;
 }
 
+// XXX: callers treat this return as boolean.
+// XXX: A better mechanism is needed to signal different types of error.
+//      lexicon, syntax, semantics, validation, access policy - are all (ab)using 'return 0'
 int
 HttpHeader::parse(const char *header_start, size_t hdrLen)
 {
@@ -346,9 +349,12 @@ HttpHeader::parse(const char *header_start, size_t hdrLen)
         const char *field_start = field_ptr;
         const char *field_end;
 
+        const char *hasBareCr = nullptr;
+        size_t lines = 0;
         do {
             const char *this_line = field_ptr;
             field_ptr = (const char *)memchr(field_ptr, '\n', header_end - field_ptr);
+            ++lines;
 
             if (!field_ptr) {
                 // missing <LF>
@@ -383,6 +389,7 @@ HttpHeader::parse(const char *header_start, size_t hdrLen)
 
             /* Barf on stray CR characters */
             if (memchr(this_line, '\r', field_end - this_line)) {
+                hasBareCr = "bare CR";
                 debugs(55, warnOnError, "WARNING: suspicious CR characters in HTTP header {" <<
                        getStringPrefix(field_start, field_end-field_start) << "}");
 
@@ -432,6 +439,18 @@ HttpHeader::parse(const char *header_start, size_t hdrLen)
             return 0;
         }
 
+        if (lines > 1 || hasBareCr) {
+            const auto framingHeader = (e->id == Http::HdrType::CONTENT_LENGTH || e->id == Http::HdrType::TRANSFER_ENCODING);
+            if (framingHeader) {
+                if (!hasBareCr) // already warned about bare CRs
+                    debugs(55, warnOnError, "WARNING: obs-fold in framing-sensitive " << e->name << ": " << e->value);
+                delete e;
+                PROF_stop(HttpHeaderParse);
+                clean();
+                return 0;
+            }
+        }
+
         if (e->id == Http::HdrType::CONTENT_LENGTH && !clen.checkField(e->value)) {
             delete e;