9ad04e0a0b06ec3c4f09ef1b6c3ce6be79b61c68 braney Sun Aug 16 11:56:56 2026 -0700 cart: validate file names read back out of the cart Several cart variables hold the name of a file the server created for a user. Route them through one shared check, isServerUserFilePath(), which accepts the trash directory, the session-data directories and myVariantsDataDir, and apply it both where values enter the cart and where the file names are used. A few of these variables may instead hold a remote URL. Those get their own list and isServerUserFileOrUrl(), because the code that reads them chooses between a fetch and a local open by looking for a protocol. Consolidates two hand-rolled copies of the same test in blatShare.c and customFactory.c, and drops the weaker private copy in sessionData.c. Adds hg/utils/cartFileVarCatalog, a registry that scans the tree for a cart value reaching a file call and reconciles what it finds against the lists in cart.c, so a new one of these cannot be added without somebody noticing. Its --reconcile is quiet enough for the nightly cron the other catalogs use, and it is what turned up seven of the names now on those lists. refs #37623 diff --git src/hg/lib/cart.c src/hg/lib/cart.c index 12d9b70b15f..c0c990defee 100644 --- src/hg/lib/cart.c +++ src/hg/lib/cart.c @@ -29,30 +29,33 @@ #include "geoMirror.h" #include "hubConnect.h" #include "trackHub.h" #include "cgiApoptosis.h" #include "customComposite.h" #include "regexHelper.h" #include "windowsToAscii.h" #include "jsonWrite.h" #include "verbose.h" #include "genark.h" #include "quickLift.h" #include "botDelay.h" #include "curlWrap.h" #include "hubSpaceKeys.h" #include "myVariantsShare.h" +#include "customTrack.h" +#include "dupTrack.h" +#include "myVariants.h" static char *sessionVar = "hgsid"; /* Name of cgi variable session is stored in. */ static char *positionCgiName = "position"; DbConnector cartDefaultConnector = hConnectCart; DbDisconnect cartDefaultDisconnector = hDisconnectCart; static boolean cartDidContentType = FALSE; struct slPair *httpHeaders = NULL; // A list of headers to output before the content-type static void hashUpdateDynamicVal(struct hash *hash, char *name, void *val) /* Val is a dynamically allocated (freeMem-able) entity to put * in hash. Override existing hash item with that name if any. * Otherwise make new hash item. */ { @@ -183,47 +186,159 @@ while ((helOverlay = hashNext(&cookie)) != NULL) { char *varName = helOverlay->name; struct hashEl *helOrig = hashLookup(origHash, varName); if (helOrig) { // we don't want to hide a track that's visible in the overlay if (differentString("hide", helOverlay->val)) hashReplace(origHash, helOverlay->name, helOverlay->val); } else hashAdd(origHash, varName, helOverlay->val); } } +/* Cart variables below hold the name of a file that the server itself made for this user, + * either in the trash directory or in the durable session-data directory that a saved + * session's trash files get moved to. Each of these names reaches an open, a read or a + * delete somewhere in the tree, and a cart value is not ours to trust: values arrive from + * CGI parameters, from an uploaded or fetched hgSession settings file, and from another + * user's shared session. So a value that names some other file is dropped on the way in, + * by cartValueIsAcceptable() below. + * + * Add a name here when a new cart variable comes to hold a server-side file name. Keep + * validating at the point of use as well, with isServerUserFilePath(): some file name + * variables are named by a trackDb setting rather than by the code (speciesUseFile), so no + * fixed list can cover them. + * + * hg/utils/cartFileVarCatalog checks these arrays for completeness: it scans the tree for a + * cart value reaching a file call and fails if it finds one that is neither listed here nor + * described in the catalog. A name added here wants a row there as well. */ + +static char *fileNameCartVars[] = +{ + "hgta_userRegionsFile", // user regions, hgTables.h hgtaUserRegionsFile, also hgIntegrator + "hgta_identifierFile", // hgTables pasted identifier list, hgTables.h hgtaIdentifierFile + DUP_TRACKS_VAR, // duplicated track stanzas + "blatLastBigBed", // BLAT bigPsl, see blatFindPinnedBigPsl() + "blatPslFile", // saved BLAT result, see hgBlat doRedisplayResults() + "blatFaFile", // the query sequence that goes with blatPslFile + "hgg_mrnaFoldPs", // hgGene mRNA fold PostScript, hgGene.h hggMrnaFoldPs + "hgp_matchFile", // hgVisiGene search matches, hgVisiGene.h hgpMatchFile + "near.customFile", // hgNear custom column file, hgNear.h customFileVarName + "gsTemp", // the file hgTables uploads to GenomeSpace +}; + +static char *fileNameCartVarPrefixes[] = +{ + CT_FILE_VAR_PREFIX, // ctfile_<db>, ctfile_hub_<id>: custom tracks + MYVARIANTS_FILE_VAR_PREFIX, // mvCtfile_<db>: myVariants custom track + customCompositeCartName "-", // customComposite-<db>: track collection hub + quickLiftCartName "-", // hubQuickLift-<db>: quickLift hub +}; + +/* These two hold either a remote URL or the name of a file the server made, and the code that + * reads them tells the cases apart by looking for a protocol. A value with no protocol falls + * through to opening a local file, so it has to be one of ours; a real URL is fine. */ + +static char *urlOrFileNameCartVars[] = +{ + "multiRegionsBedUrl", // hgTracks multi-region BED: a URL, or the trash file we wrote + hgsLoadUrlName, // hgSession load settings from URL +}; + +static boolean cartVarHoldsFileName(char *var) +/* Return TRUE if var is one of the cart variables listed above. */ +{ +int i; +for (i = 0; i < ArraySize(fileNameCartVars); i++) + if (sameString(var, fileNameCartVars[i])) + return TRUE; +for (i = 0; i < ArraySize(fileNameCartVarPrefixes); i++) + if (startsWith(fileNameCartVarPrefixes[i], var)) + return TRUE; +return FALSE; +} + +static boolean cartVarHoldsUrlOrFileName(char *var) +/* Return TRUE if var is one of the cart variables that may hold either. */ +{ +int i; +for (i = 0; i < ArraySize(urlOrFileNameCartVars); i++) + if (sameString(var, urlOrFileNameCartVars[i])) + return TRUE; +return FALSE; +} + +static void logDroppedFileNameVar(char *var, char *why) +/* Note the drop in the error log, so that a false positive can be spotted after release. + * The variable name comes from the user, so copy out only characters that cannot forge a + * log line of their own, and keep the copy short. */ +{ +char clean[129]; +int i; +for (i = 0; i < (int)sizeof(clean) - 1 && var[i] != 0; i++) + { + unsigned char c = var[i]; + clean[i] = (isalnum(c) || c == '_' || c == '.' || c == '-') ? c : '?'; + } +clean[i] = 0; +fprintf(stderr, "cart: dropped %s, value is not %s\n", clean, why); +} + +static boolean cartValueIsAcceptable(char *var, char *val) +/* Return TRUE unless var names a server-created file and val names something else. An empty + * value is fine; the CGIs treat it as "no file" and several saved sessions carry one. */ +{ +if (isEmpty(val)) + return TRUE; +if (cartVarHoldsFileName(var)) + { + if (isServerUserFilePath(val)) + return TRUE; + logDroppedFileNameVar(var, "a trash or session-data file name"); + return FALSE; + } +if (cartVarHoldsUrlOrFileName(var)) + { + if (isServerUserFileOrUrl(val)) + return TRUE; + logDroppedFileNameVar(var, "a URL or a trash or session-data file name"); + return FALSE; + } +return TRUE; +} + static void loadHash(struct hash *hash, char *contents) /* Load a hash from a cart-like string. */ { char *namePt, *dataPt, *nextNamePt; namePt = contents; while (namePt != NULL && namePt[0] != 0) { dataPt = strchr(namePt, '='); if (dataPt == NULL) errAbort("Mangled input string %s", namePt); *dataPt++ = 0; nextNamePt = strchr(dataPt, '&'); if (nextNamePt == NULL) nextNamePt = strchr(dataPt, ';'); /* Accomodate DAS. */ if (nextNamePt != NULL) *nextNamePt++ = 0; cgiDecode(dataPt,dataPt,strlen(dataPt)); + if (cartValueIsAcceptable(namePt, dataPt)) hashAdd(hash, namePt, cloneString(dataPt)); namePt = nextNamePt; } } void cartParseOverHashExt(struct cart *cart, char *contents, boolean merge) /* Parse cgi-style contents into a hash table. If merge is FALSE, this will *not* * replace existing members of hash that have same name, so we can * support multi-select form inputs (same var name can have multiple * values which will be in separate hashEl's). If merge is TRUE, we * replace existing values with new values */ { if (merge) { struct hash *newHash = newHash(8); @@ -589,30 +704,32 @@ char *multVar = cv->name + multSize; if (! cgiVarExists(multVar)) { storeInOldVars(cart, oldVars, multVar); storeInOldVars(cart, oldVars, cv->name); cartRemove(cart, multVar); } } } /* Handle non-boolean vars. */ for (cv = cgiVarList(); cv != NULL; cv = cv->next) { if (! (startsWith(booShadow, cv->name) || hashLookup(booHash, cv->name))) { + if (!cartValueIsAcceptable(cv->name, cv->val)) + continue; // leave the file name the server itself stored in the cart alone storeInOldVars(cart, oldVars, cv->name); cartRemove(cart, cv->name); if (differentString(cv->val, CART_VAR_EMPTY)) // NOTE: CART_VAR_EMPTY logic not implemented for boolShad hashAdd(cgiHash, cv->name, cv->val); } } /* Add new settings to cart (old values of these variables have been * removed above). */ struct hashEl *hel = hashElListHash(cgiHash); while (hel != NULL) { cartAddString(cart, hel->name, hel->val); hel = hel->next; } @@ -1234,30 +1351,31 @@ } if (addToCart) { if (val != NULL) { size_t valLen = strlen(val); if (valLen > CART_VAL_MAX_LENGTH) { addToCart = FALSE; vsGotValTooLong(stats, var, valLen); } else { if (decodeVal) decodeForHgSession(val); + if (cartValueIsAcceptable(var, val)) cartAddString(cart, var, val); updatePrevVar(pPrevVar, var); } } else if (var != NULL) { cartSetString(cart, var, ""); updatePrevVar(pPrevVar, var); } } return addToCart; } boolean cartLoadSettingsFromUserInput(struct lineFile *lf, struct cart *cart, struct hash *oldVars, char *actionVar, struct dyString *dyMessage) @@ -1741,30 +1859,34 @@ if (cartVarExists(cart, hgsDoOtherUser)) { char *otherUser = cartString(cart, hgsOtherUserName); char *sessionName = cartString(cart, hgsOtherUserSessionName); boolean mergeCart = cartUsualBoolean(cart, hgsMergeCart, FALSE); struct sqlConnection *conn2 = hConnectCentral(); cartLoadUserSessionExt(conn2, otherUser, sessionName, cart, oldVars, hgsDoOtherUser, mergeCart); hDisconnectCentral(&conn2); cartTrace(cart, "after cartLUS", conn); didSessionLoad = TRUE; } else if (cartVarExists(cart, hgsDoLoadUrl)) { char *url = cartString(cart, hgsLoadUrlName); + /* netUrlOpen() treats a string with no protocol as a local path and open()s it + * (lib/net.c), so this has to be a URL or a file we made before it is opened. */ + if (!isServerUserFileOrUrl(url)) + errAbort("Can only load session settings from a URL."); struct lineFile *lf = netLineFileOpen(url); struct dyString *dyMessage = dyStringNew(0); boolean ok = cartLoadSettingsFromUserInput(lf, cart, oldVars, hgsDoLoadUrl, dyMessage); lineFileClose(&lf); cartTrace(cart, "after cartLS", conn); if (! ok) { warn("Unable to load session file: %s", dyMessage->string); } didSessionLoad = ok; dyStringFree(&dyMessage); } } #endif /* GBROWSE */ @@ -3152,30 +3274,35 @@ cartDefaultDisconnector = disconnector; } char *cartGetOrderFromFile(char *genomeDb, struct cart *cart, char *speciesUseFile) /* Look in a cart variable that holds the filename that has a list of * species to show in a maf file */ { char *val; struct dyString *orderDY = dyStringNew(256); char *words[16]; if ((val = cartUsualString(cart, speciesUseFile, NULL)) == NULL) { errAbort("can't find species list file var '%s' in cart\n",speciesUseFile); } +/* speciesUseFile names the cart variable, and a trackDb or hub author picks that name, so no + * fixed list in the cart can screen this one -- check the value here instead. */ +if (!isServerUserFilePath(val)) + errAbort("species list file var '%s' does not name a file we made\n", speciesUseFile); + struct lineFile *lf = lineFileOpen(val, TRUE); if (lf == NULL) errAbort("can't open species list file %s",val); while( ( lineFileChopNext(lf, words, sizeof(words)/sizeof(char *)) )) dyStringPrintf(orderDY, "%s ",words[0]); return dyStringCannibalize(&orderDY); } char *cartLookUpVariableClosestToHome(struct cart *cart, struct trackDb *tdb, boolean parentLevel, char *suffix,char **pVariable) /* Returns value or NULL for a cart variable from lowest level on up. Optionally * fills the non NULL pVariable with the actual name of the variable in the cart */