58104a98358604975dac0a9350ca91d2d25c501d max Thu Sep 10 05:02:45 2026 -0700 hUserAbort shows its message to the user instead of turning into a 500 hUserAbort() reports an error caused by user input, so the message is written to be read by the user. It was only reaching them when a CGI had already pushed a warn handler of its own. The apiKey and bot checks call it from main() before that happens, and the default handler then writes to stderr and nothing else, unless hg.conf sets showEarlyErrors - off by default, and off on the RR. Apache turns the empty response into a 500, which is what hubApi/hubApi.c works around by pre-validating the apiKey itself. hVaUserAbort() now turns doContentType on for the rest of the process when it is running as a CGI, so the default handler emits the Content-Type line and the message. It stays off for a program that never called cgiSpoof(), and it is inert inside an errCatch, which pushes its own warn handler - so a caller that catches the abort to write its own response (hubApi's JSON) is unchanged. Fixes the va_list handling in defaultVaWarn() while in there. It read args three times but only the second and third read from a va_copy: the first vfprintf consumed args itself, so the two reads after it saw a spent va_list and the copy sent to the browser lost every %s and %d. It printed "Bad thing: [br]" where the message was "Bad thing: %s<br>". Every read now takes its own copy, and the buffer is filled with vsnprintf rather than vsprintf. No XSS: on this path defaultVaWarn replaces < and > with [ and ] across the whole formatted message, args included. The other handlers that can report an hUserAbort - earlyWarningHandler and cartEarlyWarningHandler via htmlVaEncodeErrorText, htmlVaWarn, webVaWarn - all run the arguments through vaHtmlDyStringPrintf, which html-encodes & < > / " and '. No caller passes user data as the format string. diff --git src/lib/errAbort.c src/lib/errAbort.c index d63b2944691..f3f5e7e70df 100644 --- src/lib/errAbort.c +++ src/lib/errAbort.c @@ -51,54 +51,59 @@ static struct perThreadAbortVars *getThreadVars(); // forward declaration static void defaultVaWarn(char *format, va_list args) /* Default error message handler. */ { if (format == NULL) return; if (doContentType) { cgiPrintContentType("text/html"); puts("Error: "); // Need to destroy < and > in format AND args, to make XSS impossible. + // args is read three times below, and a va_list cannot be read twice, so every + // read takes a fresh va_copy. Reading args itself even once spends it and + // leaves the later reads printing garbage or nothing at all. va_list args_copy; // first output message to stderr, as before - va_copy(args_copy, args); // vfprintf() & co cannot be called twice in a row without a va_copy - vfprintf(stderr, format, args); + va_copy(args_copy, args); + vfprintf(stderr, format, args_copy); va_end(args_copy); va_copy(args_copy, args); int needed = vsnprintf(NULL, 0, format, args_copy); // get size of buffer va_end(args_copy); if (needed < 0) { puts("defaultVaWarn - string format error in errAbort"); // Formatting error return; } char *buffer = malloc(needed + 1); // allocate buffer if (!buffer) { // out of mem error triggers errAbort puts("defaultVaWarn - cannot allocate memory for errAbort message. See stderr or error log for message"); return; } - vsprintf(buffer, format, args); // write message to buffer + va_copy(args_copy, args); + vsnprintf(buffer, needed + 1, format, args_copy); // write message to buffer + va_end(args_copy); for (char *p = buffer; *p; ++p) { // sanitize buffer if (*p == '<') *p = '['; if (*p == '>') *p = ']'; } fputs(buffer, stdout); // output buffer fprintf(stdout, "\n"); fflush(stdout); free(buffer); } else { // normal case, for command line tools or browsers where showEarlyWarnings is not set in hg.conf fflush(stdout);