From: Amos Jeffries <squid3@treenet.co.nz>
Date: Wed, 20 Apr 2016 11:17:31 +0000 (+1200)
Subject: Fix several ESI element construction issues
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d41dc37ddbe6a08a8e5a1d3267d1007b997b2396;p=thirdparty%2Fsquid.git

Fix several ESI element construction issues

* Do not wrap active logic in assert().

* Fix localbuf array bounds checking.

* Add Must() conditions to verify array writes will succeed
---

diff --git a/src/esi/Esi.cc b/src/esi/Esi.cc
index 083e9a88ed..474e0c791a 100644
--- a/src/esi/Esi.cc
+++ b/src/esi/Esi.cc
@@ -1007,7 +1007,7 @@ ESIContext::start(const char *el, const char **attr, size_t attrCount)
     ESIElement::Pointer element;
     int specifiedattcount = attrCount * 2;
     char *position;
-    assert (ellen < sizeof (localbuf)); /* prevent unexpected overruns. */
+    Must(ellen < sizeof(localbuf)); /* prevent unexpected overruns. */
 
     debugs(86, 5, "ESIContext::Start: element '" << el << "' with " << specifiedattcount << " tags");
 
@@ -1021,15 +1021,17 @@ ESIContext::start(const char *el, const char **attr, size_t attrCount)
         /* Spit out elements we aren't interested in */
         localbuf[0] = '<';
         localbuf[1] = '\0';
-        assert (xstrncpy (&localbuf[1], el, sizeof(localbuf) - 2));
+        xstrncpy(&localbuf[1], el, sizeof(localbuf) - 2);
         position = localbuf + strlen (localbuf);
 
         for (i = 0; i < specifiedattcount && attr[i]; i += 2) {
+            Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 1);
             *position = ' ';
             ++position;
             /* TODO: handle thisNode gracefully */
-            assert (xstrncpy (position, attr[i], sizeof(localbuf) + (position - localbuf)));
+            xstrncpy(position, attr[i], sizeof(localbuf) - (position - localbuf));
             position += strlen (position);
+            Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 2);
             *position = '=';
             ++position;
             *position = '\"';
@@ -1038,18 +1040,21 @@ ESIContext::start(const char *el, const char **attr, size_t attrCount)
             char ch;
             while ((ch = *chPtr++) != '\0') {
                 if (ch == '\"') {
-                    assert( xstrncpy(position, "&quot;", sizeof(localbuf) + (position-localbuf)) );
+                    Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 6);
+                    xstrncpy(position, "&quot;", sizeof(localbuf) - (position-localbuf));
                     position += 6;
                 } else {
+                    Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 1);
                     *position = ch;
                     ++position;
                 }
             }
-            position += strlen (position);
+            Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 1);
             *position = '\"';
             ++position;
         }
 
+        Must(static_cast<size_t>(position - localbuf) < sizeof(localbuf) - 2);
         *position = '>';
         ++position;
         *position = '\0';
@@ -1135,11 +1140,11 @@ ESIContext::end(const char *el)
     switch (ESIElement::IdentifyElement (el)) {
 
     case ESIElement::ESI_ELEMENT_NONE:
-        assert (ellen < sizeof (localbuf)); /* prevent unexpected overruns. */
+        Must(ellen < sizeof(localbuf) - 3); /* prevent unexpected overruns. */
         /* Add elements we aren't interested in */
         localbuf[0] = '<';
         localbuf[1] = '/';
-        assert (xstrncpy (&localbuf[2], el, sizeof(localbuf) - 3));
+        xstrncpy(&localbuf[2], el, sizeof(localbuf) - 3);
         position = localbuf + strlen (localbuf);
         *position = '>';
         ++position;