12a0dc62bdfc01eb3f94cc675657ab21b893c67a
braney
  Tue Aug 11 09:22:06 2026 -0700
lib: reject the reserved 0x01 escape marker in sqlSafef, htmlSafef and cgiDecode, refs #38051

sqlSafef does not escape values as it formats them. It formats first, then
escapes in a second pass. To mark which spans need escaping it wraps each
quoted %s in an in-band marker byte, escPunc = 0x01. sqlEscapeAllStrings then
pairs up the 0x01 bytes, escapes the text between a pair, and copies text
outside a pair raw.

The marker shares the byte stream with the data. A value carrying its own pair
of 0x01 bytes forges an extra pair and shifts the pairing, so part of the value
is copied raw instead of escaped. For a value of "x\x01' OR '1'='1\x01x" the
query came out as name='x' OR '1'='1x' with live quotes. cgiDecode turns %01
into a literal 0x01 and filtered nothing, so this was reachable from a plain
GET parameter with no login. htmshell.c uses the same trick for htmlSafef, where
the payoff is XSS instead of SQL.

Two guards:

vaSqlSafefNoAbort now rejects a quoted %s value that already contains escPunc.
vaHtmlSafefNoAbort counts the markers after formatting and requires exactly two
per escaped string. Both use errAbort rather than sqlCheckError or the noAbort
return. sqlCheckError honors noSqlInj.level and can be downgraded to warn and
then return, which would leave the injection live, and vaHtmlDyStringPrintf
reads a negative return as "buffer too small" and would retry forever. A raw
0x01 is never legitimate in either place, so there is no false-positive cost.

cgiDecode and cgiDecodeFull now drop 0x01, both percent-encoded and raw, so the
marker never enters a cart or CGI string. Only 0x01. Tab, newline and CR are
left alone because custom-track textarea uploads need them. Multipart uploads do
not pass through cgiDecode, so the sqlSafef and htmlSafef guards are what cover
that path.

Pre-escaped %-s arguments are not checked individually. They come either from
sqlSafef output, which now aborts before it can produce a marker, or from
sqlCheckIdentifier and sqlCkIl, which whitelist from a disallow-all table that
never permits 0x01.

Removing the in-band marker altogether is the durable fix and is tracked
separately, since it rewrites a hot path and needs its own performance testing.

diff --git src/lib/htmshell.c src/lib/htmshell.c
index a49d33bbe2d..d217e315924 100644
--- src/lib/htmshell.c
+++ src/lib/htmshell.c
@@ -1586,30 +1586,45 @@
 	    }
 	}
     ++i;	    
     }
 
 int sz = 0; 
 boolean overflow = FALSE;
 if (escStringsCount > 0)
     {
     int tempSize = bufSize + 3*escStringsCount;  // allow for temporary escPunc chars + spectype-char
     char *tempBuf = needMem(tempSize);
     sz = vsnprintf(tempBuf, tempSize, newFormat, args);
     /* note that some versions return -1 if too small */
     if (sz != -1 && sz + 1 <= tempSize)
 	{
+	/* SECURITY (refs #38051): we inserted exactly two htmlSafefPunc markers per
+	 * escaped string, so any extra one came from a value and would forge a
+	 * delimiter pair.  htmlEscapeAllStrings copies text outside a pair raw, so a
+	 * forged pair smuggles unescaped user data into the output.  Abort hard: a raw
+	 * 0x01 is never legitimate here, and callers such as vaHtmlDyStringPrintf read
+	 * a negative return as "buffer too small" and would retry forever. */
+	int puncCount = 0;
+	char *p = tempBuf;
+	while ((p = memchr(p, htmlSafefPunc, (tempBuf + sz) - p)) != NULL)
+	    {
+	    ++puncCount;
+	    ++p;
+	    }
+	if (puncCount != 2*escStringsCount)
+	    errAbort("Illegal control character in htmlSafef string value.");
 	sz = htmlEscapeAllStrings(buffer, tempBuf, bufSize, noAbort, noWarnOverflow);
 	}
     else
 	overflow = TRUE;
     freeMem(tempBuf);
     }
 else
     {
     sz = vsnprintf(buffer, bufSize, newFormat, args);
     /* note that some version return -1 if too small */
     if ((sz < 0) || (sz >= bufSize))
 	overflow = TRUE;
     }
 if (overflow)
     {