From: Amos Jeffries <amosjeffries@squid-cache.org>
Date: Mon, 1 Nov 2010 00:21:57 +0000 (-0600)
Subject: Harden quoted-string parser to RFC requirements
X-Git-Tag: take1~125
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=34460e19ab004a3cd339ef971c46b1dfcbba2a1d;p=thirdparty%2Fsquid.git

Harden quoted-string parser to RFC requirements

Fix RFC 2616 section 2.2 quote-string handling.

 * Restrict the parser to the known length of the value string to prevent
   buffer over-reads from specially crafted inputs.
 * Drop quoted-string values containing CTL octets.
---

diff --git a/src/HttpHdrSc.cc b/src/HttpHdrSc.cc
index 91fde9c01d..2e8a9ee09e 100644
--- a/src/HttpHdrSc.cc
+++ b/src/HttpHdrSc.cc
@@ -115,7 +115,7 @@ httpHdrScParseInit(HttpHdrSc * sc, const String * str)
     const char *target = NULL; /* ;foo */
     const char *temp = NULL; /* temp buffer */
     int type;
-    int ilen;
+    int ilen, vlen;
     int initiallen;
     HttpHdrScTarget *sct;
     assert(sc && str);
@@ -124,10 +124,13 @@ httpHdrScParseInit(HttpHdrSc * sc, const String * str)
 
     while (strListGetItem(str, ',', &item, &ilen, &pos)) {
         initiallen = ilen;
+        vlen = 0;
         /* decrease ilen to still match the token for  '=' statements */
 
-        if ((p = strchr(item, '=')) && (p - item < ilen))
-            ilen = p++ - item;
+        if ((p = strchr(item, '=')) && (p - item < ilen)) {
+            vlen = ilen - (++p - item);
+            ilen = p - item;
+        }
 
         /* decrease ilen to still match the token for ';' qualified non '=' statments */
         else if ((p = strchr(item, ';')) && (p - item < ilen))
@@ -194,7 +197,7 @@ httpHdrScParseInit(HttpHdrSc * sc, const String * str)
 
         case SC_CONTENT:
 
-            if (!p || !httpHeaderParseQuotedString(p, &sct->content)) {
+            if (!p || !httpHeaderParseQuotedString(p, vlen, &sct->content)) {
                 debugs(90, 2, "sc: invalid content= quoted string near '" << item << "'");
                 sct->content.clean();
                 EBIT_CLR(sct->mask, type);
diff --git a/src/HttpHeader.h b/src/HttpHeader.h
index cb1aa653f2..8d317d7dca 100644
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -276,7 +276,7 @@ private:
 };
 
 
-extern int httpHeaderParseQuotedString (const char *start, String *val);
+extern int httpHeaderParseQuotedString(const char *start, const int len, String *val);
 SQUIDCEXTERN int httpHeaderHasByNameListMember(const HttpHeader * hdr, const char *name, const char *member, const char separator);
 SQUIDCEXTERN void httpHeaderUpdate(HttpHeader * old, const HttpHeader * fresh, const HttpHeaderMask * denied_mask);
 SQUIDCEXTERN void httpHeaderCalcMask(HttpHeaderMask * mask, http_hdr_type http_hdr_type_enums[], size_t count);
diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc
index 2d33265081..5ef08389f9 100644
--- a/src/HttpHeaderTools.cc
+++ b/src/HttpHeaderTools.cc
@@ -327,11 +327,11 @@ httpHeaderParseOffset(const char *start, int64_t * value)
 /**
  * Parses a quoted-string field (RFC 2616 section 2.2), complains if
  * something went wrong, returns non-zero on success.
+ * Un-escapes quoted-pair characters found within the string.
  * start should point at the first double-quote.
- * RC TODO: This is too looose. We should honour the BNF and exclude CTL's
  */
 int
-httpHeaderParseQuotedString(const char *start, String *val)
+httpHeaderParseQuotedString(const char *start, const int len, String *val)
 {
     const char *end, *pos;
     val->clean();
@@ -341,16 +341,24 @@ httpHeaderParseQuotedString(const char *start, String *val)
     }
     pos = start + 1;
 
-    while (*pos != '"') {
+    while (*pos != '"' && len > (pos-start)) {
         bool quoted = (*pos == '\\');
         if (quoted)
             pos++;
-        if (!*pos) {
+        if (!*pos || (pos-start) > len) {
             debugs(66, 2, "failed to parse a quoted-string header field near '" << start << "'");
             val->clean();
             return 0;
         }
-        end = pos + strcspn(pos + quoted, "\"\\") + quoted;
+        end = pos;
+        while(end <= (start+len) && *end != '\\' && *end != '\"' && *end > 0x1F && *end != 0x7F)
+            end++;
+        if (*end <= 0x1F || *end == 0x7F) {
+            debugs(66, 2, "failed to parse a quoted-string header field with CTL octet " << (start-pos)
+                   << " bytes into '" << start << "'");
+            val->clean();
+            return 0;
+        }
         val->append(pos, end-pos);
         pos = end;
     }
diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc
index 4da2f632fb..90ed714b99 100644
--- a/src/auth/digest/auth_digest.cc
+++ b/src/auth/digest/auth_digest.cc
@@ -887,7 +887,7 @@ AuthDigestConfig::decode(char const *proxy_auth)
         String value;
         if (vlen > 0) {
             if (*p == '"') {
-                if (!httpHeaderParseQuotedString(p, &value)) {
+                if (!httpHeaderParseQuotedString(p, vlen, &value)) {
                     debugs(29, 9, "authDigestDecodeAuth: Failed to parse attribute '" << item << "' in '" << temp << "'");
                     continue;
                 }