commit 485c9a7bb1bba88754e07ad0094647ea57a6eb8d
Author: Alex Rousskov <rousskov@measurement-factory.com>
Date:   2020-05-13 14:05:00 +0000

    Validate Content-Length value prefix (#629)
    
    The new code detects all invalid Content-Length prefixes but the old
    code was already rejecting most invalid prefixes using strtoll(). The
    newly covered (and now rejected) invalid characters are
    
    * explicit "+" sign;
    * explicit "-" sign in "-0" values;
    * isspace(3) characters that are not (relaxed) OWS characters.
    
    In most deployment environments, the last set is probably empty because
    the relaxed OWS set has all the POSIX/C isspace(3) characters but the
    new line, and the new line is unlikely to sneak in past other checks.
    
    Thank you, Amit Klein <amit.klein@safebreach.com>, for elevating the
    importance of this 2016 TODO (added in commit a1b9ec2).

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 36957f2..c10a221 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -25,6 +25,7 @@ Thank you!
     Alex Wu <alex_wu2012@hotmail.com>
     Alin Nastac <mrness@gentoo.org>
     Alter <alter@alter.org.ua>
+    Amit Klein <amit.klein@safebreach.com>
     Amos Jeffries
     Amos Jeffries <amosjeffries@squid-cache.org>
     Amos Jeffries <squid3@treenet.co.nz>
diff --git a/src/http/ContentLengthInterpreter.cc b/src/http/ContentLengthInterpreter.cc
index 58e14e6..b3c077d 100644
--- a/src/http/ContentLengthInterpreter.cc
+++ b/src/http/ContentLengthInterpreter.cc
@@ -29,6 +29,24 @@ Http::ContentLengthInterpreter::ContentLengthInterpreter():
 {
 }
 
+/// checks whether all characters before the Content-Length number are allowed
+/// \returns the start of the digit sequence (or nil on errors)
+const char *
+Http::ContentLengthInterpreter::findDigits(const char *prefix, const char * const valueEnd) const
+{
+    // skip leading OWS in RFC 7230's `OWS field-value OWS`
+    const CharacterSet &whitespace = Http::One::Parser::WhitespaceCharacters();
+    while (prefix < valueEnd) {
+        const auto ch = *prefix;
+        if (CharacterSet::DIGIT[ch])
+            return prefix; // common case: a pre-trimmed field value
+        if (!whitespace[ch])
+            return nullptr; // (trimmed) length does not start with a digit
+        ++prefix;
+    }
+    return nullptr; // empty or whitespace-only value
+}
+
 /// checks whether all characters after the Content-Length are allowed
 bool
 Http::ContentLengthInterpreter::goodSuffix(const char *suffix, const char * const end) const
@@ -53,10 +71,19 @@ Http::ContentLengthInterpreter::checkValue(const char *rawValue, const int value
 {
     Must(!sawBad);
 
+    const auto valueEnd = rawValue + valueSize;
+
+    const auto digits = findDigits(rawValue, valueEnd);
+    if (!digits) {
+        debugs(55, debugLevel, "WARNING: Leading garbage or empty value in" << Raw("Content-Length", rawValue, valueSize));
+        sawBad = true;
+        return false;
+    }
+
     int64_t latestValue = -1;
     char *suffix = nullptr;
-    // TODO: Handle malformed values with leading signs (e.g., "-0" or "+1").
-    if (!httpHeaderParseOffset(rawValue, &latestValue, &suffix)) {
+
+    if (!httpHeaderParseOffset(digits, &latestValue, &suffix)) {
         debugs(55, DBG_IMPORTANT, "WARNING: Malformed" << Raw("Content-Length", rawValue, valueSize));
         sawBad = true;
         return false;
@@ -69,7 +96,7 @@ Http::ContentLengthInterpreter::checkValue(const char *rawValue, const int value
     }
 
     // check for garbage after the number
-    if (!goodSuffix(suffix, rawValue + valueSize)) {
+    if (!goodSuffix(suffix, valueEnd)) {
         debugs(55, debugLevel, "WARNING: Trailing garbage in" << Raw("Content-Length", rawValue, valueSize));
         sawBad = true;
         return false;
diff --git a/src/http/ContentLengthInterpreter.h b/src/http/ContentLengthInterpreter.h
index 37d9e69..142664e 100644
--- a/src/http/ContentLengthInterpreter.h
+++ b/src/http/ContentLengthInterpreter.h
@@ -67,6 +67,7 @@ public:
     bool sawGood;
 
 protected:
+    const char *findDigits(const char *prefix, const char *valueEnd) const;
     bool goodSuffix(const char *suffix, const char * const end) const;
     bool checkValue(const char *start, const int size);
     bool checkList(const String &list);