934a51aa2ebc7e11bfba57804642cd03e4e1e60a braney Tue Aug 18 11:45:11 2026 -0700 trackHub: tighten group name validation, refs #38123 diff --git src/hg/lib/trackHub.c src/hg/lib/trackHub.c index 0ada6dd9206..a37ea145888 100644 --- src/hg/lib/trackHub.c +++ src/hg/lib/trackHub.c @@ -511,46 +511,59 @@ } slFreeList(&chromList); return ciList; } static char *getRequiredGrpSetting(struct hash *hash, char *name, struct lineFile *lf) /* Grab a group setting out of the group hash. errAbort if not found. */ { char *str; if ((str = hashFindVal(hash, name)) == NULL) errAbort("missing required setting '%s' for group on line %d in file %s\n", name, lf->lineIx, lf->fileName); return str; } +static void checkHubIdName(char *type, char *name) +/* Abort if name holds a character that is not valid in an identifier. A hub + * identifier - a genome name, a group name - is printed into dozens of URLs, + * form values and attributes all over the CGIs, so the check belongs here rather than at every + * one of those places. Real names use letters, digits, underscore, dot and dash. */ +{ +if (strchr(name, '<') || strchr(name, '>') || strchr(name, '"') + || strchr(name, '\'') || strchr(name, '&')) + errAbort("Bad %s name: \"%s\". The characters < > \" ' and & are not allowed in a %s name.", + type, name, type); +} + struct grp *readGroupRa(char *groupFileName) /* Read in the ra file that describes the groups in an assembly hub. */ { if (groupFileName == NULL) return NULL; struct hash *ra; struct grp *list = NULL; struct lineFile *lf = udcWrapShortLineFile(groupFileName, NULL, MAX_HUB_GROUP_FILE_SIZE); while ((ra = raNextRecord(lf)) != NULL) { char *str; struct grp *grp; AllocVar(grp); slAddHead(&list, grp); grp->name = cloneString(getRequiredGrpSetting(ra, "name", lf)); + checkHubIdName("group", grp->name); grp->label = cloneString(getRequiredGrpSetting(ra, "label", lf)); grp->priority = BIGDOUBLE; str = hashFindVal(ra, "priority"); if (str != NULL) grp->priority = atof(str); str = hashFindVal(ra, "defaultIsClosed"); if ((str != NULL) && (sameString("on",str) || sameString("1", str))) grp->defaultIsClosed = 1; hashFree(&ra); } if (list) slSort(&list, grpCmpPriorityLabel); @@ -685,37 +698,31 @@ break; char *twoBitPath = hReplaceGbdb(hashFindVal(ra, "twoBitPath")); char *twoBitBptUrl = hReplaceGbdb(hashFindVal(ra, "twoBitBptUrl")); char *genome, *trackDb; if (twoBitPath != NULL) genome = addHubName(hashFindVal(ra, "genome"), hub->name); else genome = hashFindVal(ra, "genome"); if (hub->defaultDb == NULL) hub->defaultDb = genome; if (genome == NULL) badGenomeStanza(lf); if (hasWhiteSpace(genome)) errAbort("Bad genome name: \"%s\". Only alpha-numeric characters and \"_\" are allowed ([A-Za-z0-9_]).", genome); - // The genome name becomes the db name, which is printed into dozens of URLs and form - // values all over the CGIs. Real assembly names never contain any of these characters, - // so reject them here. - if (strchr(genome, '<') || strchr(genome, '>') || strchr(genome, '"') - || strchr(genome, '\'') || strchr(genome, '&')) - errAbort("Bad genome name: \"%s\". The characters < > \" ' and & are not allowed in a " - "genome name.", genome); + checkHubIdName("genome", genome); if (hashLookup(hash, genome) != NULL) errAbort("Duplicate genome %s in stanza ending line %d of %s", genome, lf->lineIx, lf->fileName); if (singleFile == NULL) { trackDb = hashFindVal(ra, "trackDb"); if (trackDb == NULL) badGenomeStanza(lf); } else trackDb = singleFile; AllocVar(el); el->name = cloneString(genome); el->trackDbFile = trackHubRelativeUrl(url, trackDb); el->trackHub = hub;