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/cheapcgi.c src/lib/cheapcgi.c index 0aaa8ebd1db..5e972091198 100644 --- src/lib/cheapcgi.c +++ src/lib/cheapcgi.c @@ -1349,76 +1349,92 @@ /* NOTE: Where in the URL to use which of these functions: * * Parts of a URL: * protocol://user:password@server.com:port/path/filename?var1=val1&var2=val2 * * Note that a space should only be encoded to a plus and decoded from a plus * when dealing with http URLs in the query part of the string, * which is the part after the ? above. * It should not be used in the rest of the URL. * So in the query string part of a URL, do use cgiEncode/cgiDecode. * And in the rest of the URL, use cgiEncodeFUll/cgiDecodeFull * which do not code space as plus. * Since FTP does not use URLs with query parameters, use the Full version. */ +/* SECURITY (refs #38051): 0x01 is the in-band marker that sqlSafef (jksql.c) and + * htmlSafef (htmshell.c) use to delimit the values they must escape. A request + * value carrying this byte can forge a delimiter pair and smuggle unescaped text + * into a query or into page output, so drop it here as it is decoded. It is + * never legitimate in a request. Only 0x01 - tab, newline and CR are left alone, + * since those are legitimate in custom-track textarea uploads. */ +#define CGI_ESCAPE_MARKER 0x01 + void cgiDecode(char *in, char *out, int inLength) /* Decode from cgi pluses-for-spaces format to normal. * Out will be a little shorter than in typically, and * can be the same buffer. */ { char c; int i; for (i=0; i<inLength;++i) { c = *in++; if (c == '+') *out++ = ' '; else if (c == '%') { int code; if (sscanf(in, "%2x", &code) != 1) code = '?'; in += 2; i += 2; + if (code == CGI_ESCAPE_MARKER) + continue; // drop our reserved escape marker *out++ = code; } + else if (c == CGI_ESCAPE_MARKER) + continue; // drop our reserved escape marker else *out++ = c; } *out++ = 0; } void cgiDecodeFull(char *in, char *out, int inLength) /* Out will be a cgi-decoded version of in (no space from plus!). * Out will be a little shorter than in typically, and * can be the same buffer. */ { char c; int i; for (i=0; i<inLength;++i) { c = *in++; if (c == '%') { int code; if (sscanf(in, "%2x", &code) != 1) code = '?'; in += 2; i += 2; + if (code == CGI_ESCAPE_MARKER) + continue; // drop our reserved escape marker *out++ = code; } + else if (c == CGI_ESCAPE_MARKER) + continue; // drop our reserved escape marker else *out++ = c; } *out++ = 0; } char *cgiEncode(char *inString) /* Return a cgi-encoded version of inString. * Alphanumerics kept as is, space translated to plus, * and all other characters translated to %hexVal. */ { char c; int outSize = 0; char *outString, *out, *in;