------------------------------------------------------------ revno: 11841 revision-id: squid3@treenet.co.nz-20160420111731-wv2byqylavvdmsp9 parent: squid3@treenet.co.nz-20160420101148-74r6kfp496g9o5e0 committer: Amos Jeffries branch nick: 3.2 timestamp: Wed 2016-04-20 23:17:31 +1200 message: 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 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20160420111731-wv2byqylavvdmsp9 # target_branch: http://bzr.squid-cache.org/bzr/squid3/branches\ # /SQUID_3_2 # testament_sha1: 08959375e80cc0ddda46b3e885c11f6f71eda542 # timestamp: 2016-04-20 11:27:19 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.2 # base_revision_id: squid3@treenet.co.nz-20160420101148-\ # 74r6kfp496g9o5e0 # # Begin patch === modified file 'src/esi/Esi.cc' --- src/esi/Esi.cc 2012-07-28 05:38:50 +0000 +++ src/esi/Esi.cc 2016-04-20 11:17:31 +0000 @@ -1007,7 +1007,7 @@ 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 @@ /* 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(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(position - localbuf) < sizeof(localbuf) - 2); *position = '='; ++position; *position = '\"'; @@ -1038,18 +1040,21 @@ char ch; while ((ch = *chPtr++) != '\0') { if (ch == '\"') { - assert( xstrncpy(position, """, sizeof(localbuf) + (position-localbuf)) ); + Must(static_cast(position - localbuf) < sizeof(localbuf) - 6); + xstrncpy(position, """, sizeof(localbuf) - (position-localbuf)); position += 6; } else { + Must(static_cast(position - localbuf) < sizeof(localbuf) - 1); *position = ch; ++position; } } - position += strlen (position); + Must(static_cast(position - localbuf) < sizeof(localbuf) - 1); *position = '\"'; ++position; } + Must(static_cast(position - localbuf) < sizeof(localbuf) - 2); *position = '>'; ++position; *position = '\0'; @@ -1135,11 +1140,11 @@ 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;