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
". 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/hg/lib/hCommon.c src/hg/lib/hCommon.c index 41e865de9e5..8768a57f18b 100644 --- src/hg/lib/hCommon.c +++ src/hg/lib/hCommon.c @@ -1,27 +1,28 @@ /* hCommon.c - routines used by many files in hgap project. */ /* Copyright (C) 2014 The Regents of the University of California * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */ #include "common.h" #include "hCommon.h" #include "chromInfo.h" #include "portable.h" #include "hgConfig.h" #include "errAbort.h" #include "htmshell.h" +#include "cheapcgi.h" static char *_hgcName = "../cgi-bin/hgc"; /* Path to click processing program. */ static char *_hgTracksName = "../cgi-bin/hgTracks"; /* Path back to genome browser. */ static char *_hgTrackUiName = "../cgi-bin/hgTrackUi"; /* Path to extended ui program. */ static char *_hgFileUiName = "../cgi-bin/hgFileUi"; /* Path to downloladable files CGI. */ static char *_hgTextName = "../cgi-bin/hgText"; /* Path back to the text browser. */ static char *_hgTablesName = "../cgi-bin/hgTables"; /* Path back to the table browser. */ static char *_hgVaiName = "../cgi-bin/hgVai"; /* Path back to the variant annotation integrator. */ static char *_hgCustomName = "../cgi-bin/hgCustom"; /* Path back to the custom tracks manager. */ static char *_hgCollectionName = "../cgi-bin/hgCollection"; /* Path back to the composite builder */ static char *_hgHubConnectName = "../cgi-bin/hgHubConnect"; /* Path back to the track hub manager. */ static char *_hgSessionName = "../cgi-bin/hgSession"; /* Path to session manager. */ static char *_hgPalName = "../cgi-bin/hgPal"; /* Path back to the protein aligner */ static char *_hgVarAnnogratorName = "../cgi-bin/hgVarAnnogrator"; /* Path to variant annot intgr */ @@ -389,30 +390,43 @@ } void hDumpStackPopAbortHandler() /* pop the stack dump abort handler from the stack if it's enabled */ { if (hDumpStackEnabled() && !hDumpAbortCalled) popAbortHandler(); hDumpAbortCalled = FALSE; } void hVaUserAbort(char *format, va_list args) /* errAbort when a `user' error is detected. This is an error that comes * from user input. This disables the logging stack dumps. */ { hDumpStackDisallow(); +/* A user error is written for the user to read, so it has to reach the browser. When we are + * called before the CGI has pushed a warn handler of its own - the apiKey and bot checks do + * this, from main() - the default handler writes only to stderr unless doContentType is set, + * and apache turns that empty response into a 500. Turn it on so the default handler emits + * the Content-Type line and the message (with < and > neutered) to stdout. This is inert + * inside an errCatch, which pushes its own warn handler, so a caller that means to catch the + * abort and write its own response (hubApi's JSON) still gets to. + * + * cgiIsOnWeb() is the right test here even though it is TRUE for a spoofed command-line run + * of a CGI: the question is "is this a CGI process", and a spoofed run wants the header too. + * A program that never calls cgiSpoof() reads FALSE and keeps writing to stderr only. */ +if (cgiIsOnWeb()) + errAbortSetDoContentType(TRUE); vaErrAbort(format, args); } void hUserAbort(char *format, ...) /* errAbort when a `user' error is detected. This is an error that comes * from user input. This disables the logging stack dumps. */ { va_list args; va_start(args, format); hVaUserAbort(format, args); va_end(args); } boolean hAllowAllTables(void) /* Return TRUE if hg.conf's hgta.disableAllTables doesn't forbid an 'all tables' menu. */