6ba4e724f84f86544c11a90f3802fd6318cd154f angie Fri Dec 21 16:28:42 2018 -0800 Watch out for invalid data and unencoded newlines when reading in file uploaded as a saved session. refs #22638, #22637 cartLoadSettings did absolutely no checking that its input looked anything like cart settings, resulting in giant useless carts in which custom track files, hgTracks HTML and other arbitrary files apparently were parsed as cart settings. Now we do quite a bit of checking in cartLoadSettingsFromUserInput, and can warn the user if it looks like they are attempting to upload a custom track as a saved session. The unencoded newlines are our fault -- since 2010, we have been writing out saved session files without encoding newlines in setting values (pasted in by the user). Then those newlines can be mistaken for the newlines that we use to separate settings. Oops! Use a heuristic that holds up so far to determine whether the newlines should have been encoded: they seem to occur in specific variables (like hgta_pastedIdentifiers), and those variables always appear in the middle of an alphabetically sorted cluster of variables with the same prefix (like hgta_). And of course, we now write out saved session files with encoded newlines, which we decode when reading back in. We even have to deal with carts that have mixed types of newlines from various garbage sources, using lineFileCarefulNewlines(). diff --git src/hg/inc/cart.h src/hg/inc/cart.h index 0ab3ef9..a6aa87c 100644 --- src/hg/inc/cart.h +++ src/hg/inc/cart.h @@ -303,38 +303,37 @@ * otherwise default. */ void cartMakeCheckBox(struct cart *cart, char *var, boolean defaultVal); /* Make a check box filled with value from cart if it exists or * default value otherwise. */ void cartMakeRadioButton(struct cart *cart, char *var, char *val, char *defaultVal); /* Make a radio button that is selected if cart variable exists and matches * value (or value matches default val if cart var doesn't exist). */ void cartSaveSession(struct cart *cart); /* Save session in a hidden variable. This needs to be called * somewhere inside of form or bad things will happen. */ void cartDump(struct cart *cart); -/* Dump contents of cart. */ +/* Dump contents of cart with anti-XSS HTML encoding. */ -void cartDumpNoEncode(struct cart *cart); -/* Dump contents of cart without HTML encoding. */ +void cartDumpHgSession(struct cart *cart); +/* Dump contents of cart with escaped newlines for hgSession output files. + * Cart variable "cartDumpAsTable" is ignored. */ #define CART_DUMP_AS_TABLE "cartDumpAsTable" -void cartDumpList(struct hashEl *elList,boolean asTable, boolean doHtmlEncode); -/* Dump list of cart variables optionally as a table with ajax update support. */ void cartDumpPrefix(struct cart *cart, char *prefix); /* Dump all cart variables with prefix */ void cartDumpLike(struct cart *cart, char *wildcard); /* Dump all cart variables matching wildcard */ struct hashEl *cartFindPrefix(struct cart *cart, char *prefix); /* Return list of name/val pairs from cart where name starts with * prefix. Free when done with hashElFreeList. */ struct hashEl *cartFindLike(struct cart *cart, char *wildCard); /* Return list of name/val pairs from cart where name matches * wildcard. Free when done with hashElFreeList. */ @@ -460,37 +459,46 @@ #define namedSessionTable "namedSessionDb" void sessionTouchLastUse(struct sqlConnection *conn, char *encUserName, char *encSessionName); /* Increment namedSessionDb.useCount and update lastUse for this session. */ void cartLoadUserSession(struct sqlConnection *conn, char *sessionOwner, char *sessionName, struct cart *cart, struct hash *oldVars, char *actionVar); /* If permitted, load the contents of the given user's session, and then * reload the CGI settings (to support override of session settings). * If non-NULL, oldVars will contain values overloaded when reloading CGI. * If non-NULL, actionVar is a cartRemove wildcard string specifying the * CGI action variable that sent us here. */ -void cartLoadSettings(struct lineFile *lf, struct cart *cart, - struct hash *oldVars, char *actionVar); -/* Load settings (cartDump output) into current session, and then - * reload the CGI settings (to support override of session settings). +boolean cartLoadSettingsFromUserInput(struct lineFile *lf, struct cart *cart, struct hash *oldVars, + char *actionVar, struct dyString *dyMessage); +/* Verify that the user data in lf looks like valid settings (hgSession saved file; + * like cartDump output, but values may or may not be htmlEncoded). + * Older session files may have unencoded newlines, causing bogus variables; + * watch out for those after pasted input variables like hgta_pastedIdentifiers. + * Users have uploaded custom tracks, DTC genotypes, hgTracks HTML, even + * binary data files. Look for problematic patterns observed in the past. + * Load settings into current session, and then reload the CGI settings + * (to support override of session settings). * If non-NULL, oldVars will contain values overloaded when reloading CGI. * If non-NULL, actionVar is a cartRemove wildcard string specifying the - * CGI action variable that sent us here. */ + * CGI action variable that sent us here. + * If input contains suspect data, then add diagnostics to dyMessage. If input + * contains so much garbage that we shouldn't even try to load what passes the filters, + * return FALSE. */ 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 *cartGetOrderFromFileAndMsaTable(char *genomeDb, struct cart *cart, char *speciesUseFile, char *msaTable); /* Look in a cart variable that holds the filename that has a list of * species to show in a maf file */ 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: subtrackName.suffix, then compositeName.view.suffix, then compositeName.suffix Optionally fills the non NULL pVariable with the actual name of the variable in the cart */ #define cartOptionalStringClosestToHome(cart,tdb,parentLevel,suffix) \