44ccfacbe3a3d4b300f80d48651c77837a4b571e
galt
  Tue Apr 26 11:12:02 2022 -0700
SQL INJECTION Prevention Version 2 - this improves our methods by making subclauses of SQL that get passed around be both easy and correct to use. The way that was achieved was by getting rid of the obscure and not well used functions sqlSafefFrag and sqlDyStringPrintfFrag and replacing them with the plain versions of those functions, since these are not needed anymore. The new version checks for NOSQLINJ in unquoted %-s which is used to include SQL clauses, and will give an error the NOSQLINJ clause is not present, and this will automatically require the correct behavior by developers. sqlDyStringPrint is a very useful function, however because it was not enforced, users could use various other dyString functions and they operated without any awareness or checking for SQL correct use. Now those dyString functions are prohibited and it will produce an error if you try to use a dyString function on a SQL string, which is simply detected by the presence of the NOSQLINJ prefix.

diff --git src/hg/lib/hdb.c src/hg/lib/hdb.c
index a776e2f..e7b26a3 100644
--- src/hg/lib/hdb.c
+++ src/hg/lib/hdb.c
@@ -557,43 +557,49 @@
  * has no chromInfo. */
 {
 if (trackHubDatabase(db))
     return trackHubDefaultChrom(db);
 
 static struct hash *hash = NULL;
 struct hashEl *hel = NULL;
 
 if (hash == NULL)
     hash = hashNew(0);
 hel = hashStore(hash, db);
 if (hel->val == NULL)
     {
     struct sqlConnection *conn = hAllocConn(db);
     if (sqlTableExists(conn, "chromInfo"))
-	hel->val = sqlQuickString(conn, NOSQLINJ "select chrom from chromInfo limit 1");
+	{
+	char query[1024];
+	sqlSafef(query, sizeof query, "select chrom from chromInfo limit 1");
+	hel->val = sqlQuickString(conn, query);
+	}
     hFreeConn(&conn);
     }
 return hel->val;
 }
 
 int hChromCount(char *db)
 /* Return the number of chromosomes (scaffolds etc.) in the given db. */
 {
 if (trackHubDatabase(db))
     return trackHubChromCount(db);
 struct sqlConnection *conn = hAllocConn(db);
-int count = sqlQuickNum(conn, NOSQLINJ "select count(*) from chromInfo");
+char query[1024];
+sqlSafef(query, sizeof query, "select count(*) from chromInfo");
+int count = sqlQuickNum(conn, query);
 hFreeConn(&conn);
 return count;
 }
 
 // This maps db names to the latest NCBI RefSeq assembly+annotation GCF_ IDs as of
 // 12/19/2017.
 struct dbToGcf
     {
     char *db;
     char *gcf;
     };
 static struct dbToGcf dbToGcf[] =
     {
     { "hg38", "GCF_000001405.37" },
     { "hg19", "GCF_000001405.25" },
@@ -1238,31 +1244,31 @@
 char fileName[HDB_MAX_PATH_STRING];
 struct dnaSeq *block = NULL;
 struct dnaSeq *bedSeq = NULL;
 int i = 0 ;
 assert(bed);
 /* Handle very simple beds and beds with blocks. */
 if(bed->blockCount == 0)
     {
     bedSeq = hChromSeq(db, bed->chrom, bed->chromStart, bed->chromEnd);
     freez(&bedSeq->name);
     bedSeq->name = cloneString(bed->name);
     }
 else
     {
     int offSet = bed->chromStart;
-    struct dyString *currentSeq = newDyString(2048);
+    struct dyString *currentSeq = dyStringNew(2048);
     hNibForChrom(db, bed->chrom, fileName);
     for(i=0; i<bed->blockCount; i++)
 	{
 	block = hFetchSeq(fileName, bed->chrom,
 			  offSet+bed->chromStarts[i], offSet+bed->chromStarts[i]+bed->blockSizes[i]);
 	dyStringAppendN(currentSeq, block->dna, block->size);
 	dnaSeqFree(&block);
 	}
     AllocVar(bedSeq);
     bedSeq->name = cloneString(bed->name);
     bedSeq->dna = cloneString(currentSeq->string);
     bedSeq->size = strlen(bedSeq->dna);
     dyStringFree(&currentSeq);
     }
 if(bed->strand[0] == '-')
@@ -1388,31 +1394,33 @@
 {
 int size = hChromSize(db, chromName);
 return hDnaFromSeq(db, chromName, 0, size, dnaLower);
 }
 
 struct slName *hAllChromNames(char *db)
 /* Get list of all chromosome names in database. */
 {
 if (trackHubDatabase(db))
     return trackHubAllChromNames(db);
 struct slName *list = NULL;
 struct sqlConnection *conn = hAllocConn(db);
 struct sqlResult *sr;
 char **row;
 
-sr = sqlGetResult(conn, NOSQLINJ "select chrom from chromInfo");
+char query[1024];
+sqlSafef(query, sizeof query, "select chrom from chromInfo");
+sr = sqlGetResult(conn, query);
 while ((row = sqlNextRow(sr)) != NULL)
     {
     struct slName *el = slNameNew(row[0]);
     slAddHead(&list, el);
     }
 sqlFreeResult(&sr);
 hFreeConn(&conn);
 return list;
 }
 
 char *hReplaceGbdbLocal(char* fileName)
  /* Returns a gbdb filename, potentially rewriting it according to hg.conf's gbdbLoc1 */
  /* Result has to be freed */
 {
 if (fileName==NULL)
@@ -2112,31 +2120,31 @@
     sqlSafef(query, sizeof(query), "select d.name from %s d, %s g "
           "where g.acc = \"%s\" "
           "and g.description = d.id", descriptionTable, gbCdnaInfoTable, accId);
     desc = sqlQuickString(conn, query);
     }
 hFreeConn(&conn);
 return desc;
 }
 
 struct bed *hGetCtBedRange(char *db, char *browserDb, char *table, char *chrom, int chromStart,
 			   int chromEnd, char *sqlConstraints)
 /* Return a bed list of all items (that match sqlConstraints, if nonNULL)
  * in the given range in table.  If chromEnd is 0, omit the range (whole chrom).
  * WARNING: this does not use the bin column and maybe slower than you would like. */
 {
-struct dyString *query = newDyString(512);
+struct dyString *query = dyStringNew(512);
 struct sqlConnection *conn = hAllocConn(db);
 struct sqlResult *sr;
 struct hTableInfo *hti;
 struct bed *bedList=NULL, *bedItem;
 char **row;
 char parsedChrom[HDB_MAX_CHROM_STRING];
 char rootName[256];
 char fullTableName[256];
 char rangeStr[1024];
 int count;
 boolean canDoUTR, canDoIntrons;
 boolean useSqlConstraints = sqlConstraints != NULL && sqlConstraints[0] != 0;
 char tStrand = '?', qStrand = '?';
 int i;
 boolean gotWhere = FALSE;
@@ -2152,74 +2160,80 @@
 	     rootName, table);
 if (hti->isSplit)
     safef(fullTableName, sizeof(fullTableName), "%s_%s", chrom, hti->rootName);
 else
     safef(fullTableName, sizeof(fullTableName), "%s", hti->rootName);
 canDoUTR = hti->hasCDS;
 canDoIntrons = hti->hasBlocks;
 
 dyStringClear(query);
 // row[0], row[1] -> start, end
 sqlDyStringPrintf(query, "SELECT %s,%s", hti->startField, hti->endField);
 // row[2] -> name or placeholder
 if (hti->nameField[0] != 0)
     sqlDyStringPrintf(query, ",%s", hti->nameField);
 else
-    dyStringAppend(query, ",0");
+    sqlDyStringPrintf(query, ",0");
 // row[3] -> score or placeholder
 if (hti->scoreField[0] != 0)
     sqlDyStringPrintf(query, ",%s", hti->scoreField);
 else
-    dyStringAppend(query, ",0");
+    sqlDyStringPrintf(query, ",0");
 // row[4] -> strand or placeholder
 if (hti->strandField[0] != 0)
     sqlDyStringPrintf(query, ",%s", hti->strandField);
 else
-    dyStringAppend(query, ",0");
+    sqlDyStringPrintf(query, ",0");
 // row[5], row[6] -> cdsStart, cdsEnd or placeholders
 if (hti->cdsStartField[0] != 0)
     sqlDyStringPrintf(query, ",%s,%s", hti->cdsStartField, hti->cdsEndField);
 else
-    dyStringAppend(query, ",0,0");
+    sqlDyStringPrintf(query, ",0,0");
 // row[7], row[8], row[9] -> count, starts, ends/sizes or empty.
 if (hti->startsField[0] != 0)
     sqlDyStringPrintf(query, ",%s,%s,%s", hti->countField, hti->startsField,
 		   hti->endsSizesField);
 else
-    dyStringAppend(query, ",0,0,0");
+    sqlDyStringPrintf(query, ",0,0,0");
 // row[10] -> tSize for PSL '-' strand coord-swizzling only:
 if (sameString("tStarts", hti->startsField))
-    dyStringAppend(query, ",tSize");
+    sqlDyStringPrintf(query, ",tSize");
 else
-    dyStringAppend(query, ",0");
+    sqlDyStringPrintf(query, ",0");
 sqlDyStringPrintf(query, " FROM %s", fullTableName);
 if (chromEnd != 0)
     {
     sqlDyStringPrintf(query, " WHERE %s < %d AND %s > %d",
 		   hti->startField, chromEnd, hti->endField, chromStart);
     gotWhere = TRUE;
     }
 if (hti->chromField[0] != 0)
     {
-    sqlDyStringPrintf(query, " %s %s = '%s'",
-		   (gotWhere ? "AND" : "WHERE"), hti->chromField, chrom);
+    if (gotWhere)
+	sqlDyStringPrintf(query, " AND ");
+    else
+	sqlDyStringPrintf(query, " WHERE ");
+    sqlDyStringPrintf(query, "%s = '%s'", hti->chromField, chrom);
     gotWhere = TRUE;
     }
 if (useSqlConstraints)
     {
-    dyStringPrintf(query, " %s %s",
-		   (gotWhere ? "AND" : "WHERE"), sqlConstraints);
+    if (gotWhere)
+	sqlDyStringPrintf(query, " AND ");
+    else
+	sqlDyStringPrintf(query, " WHERE ");
+    sqlDyStringPrintf(query, "%-s", sqlConstraints);
     gotWhere = TRUE;
     }
 
 sr = sqlGetResult(conn, query->string);
 
 while ((row = sqlNextRow(sr)) != NULL)
     {
     AllocVar(bedItem);
     bedItem->chrom      = cloneString(chrom);
     bedItem->chromStart = atoi(row[0]);
     bedItem->chromEnd   = atoi(row[1]);
     if (hti->nameField[0] != 0)
 	bedItem->name   = cloneString(row[2]);
     else
 	{
@@ -2364,69 +2378,75 @@
 			   int chromEnd, char *sqlConstraints)
 /* Return a bed list of all items (that match sqlConstraints, if nonNULL)
  * in the given range in table.  If chromEnd is 0, omit the range (whole chrom).
  * WARNING: this does not use the bin column and maybe slower than you would like. */
 {
 return hGetCtBedRange(db, NULL, table, chrom, chromStart, chromEnd, sqlConstraints);
 }
 
 int hGetBedRangeCount(char *db, char *table, char *chrom, int chromStart,
 			 int chromEnd, char *sqlConstraints)
 /* Return a count of all the items (that match sqlConstraints, if nonNULL)
  * in the given range in table.  If chromEnd is 0, omit the range (whole chrom).
  * WARNING: this does not use the bin column and maybe slower than you would like.
  * C.f. hGetBedRange() but returns only the result of SELECT COUNT(*) FROM ...  */
 {
-struct dyString *query = newDyString(512);
+struct dyString *query = dyStringNew(512);
 struct sqlConnection *conn = hAllocConn(db);
 struct hTableInfo *hti;
 char parsedChrom[HDB_MAX_CHROM_STRING];
 char rootName[256];
 char fullTableName[256];
 int count;
 boolean useSqlConstraints = sqlConstraints != NULL && sqlConstraints[0] != 0;
 boolean gotWhere = FALSE;
 
 /* Caller can give us either a full table name or root table name. */
 hParseTableName(db, table, rootName, parsedChrom);
 hti = hFindTableInfo(db, chrom, rootName);
 if (hti == NULL)
     errAbort("Could not find table info for table %s (%s)",
 	     rootName, table);
 if (hti->isSplit)
     safef(fullTableName, sizeof(fullTableName), "%s_%s", chrom, rootName);
 else
     safef(fullTableName, sizeof(fullTableName), "%s", rootName);
 
 dyStringClear(query);
 sqlDyStringPrintf(query, "SELECT count(*) FROM %s", fullTableName);
 if (chromEnd != 0)
     {
     sqlDyStringPrintf(query, " WHERE %s < %d AND %s > %d",
 		   hti->startField, chromEnd, hti->endField, chromStart);
     gotWhere = TRUE;
     }
 if (hti->chromField[0] != 0)
     {
-    sqlDyStringPrintf(query, " %s %s = '%s'",
-		   (gotWhere ? "AND" : "WHERE"), hti->chromField, chrom);
+    if (gotWhere)
+	sqlDyStringPrintf(query, " AND ");
+    else
+	sqlDyStringPrintf(query, " WHERE ");
+    sqlDyStringPrintf(query, "%s = '%s'", hti->chromField, chrom);
     gotWhere = TRUE;
     }
 if (useSqlConstraints)
     {
-    dyStringPrintf(query, " %s %s",
-		   (gotWhere ? "AND" : "WHERE"), sqlConstraints);
+    if (gotWhere)
+	sqlDyStringPrintf(query, " AND ");
+    else
+	sqlDyStringPrintf(query, " WHERE ");
+    sqlDyStringPrintf(query, "%-s", sqlConstraints);
     gotWhere = TRUE;
     }
 
 count = sqlQuickNum(conn, query->string);
 
 dyStringFree(&query);
 hFreeConn(&conn);
 return count;
 }
 
 struct bed *hGetFullBed(char *db, char *table)
 /* Return a genome-wide bed list of the table. */
 /* WARNING: This isn't designed for CGI use. It's a looped call to */
 /* hGetBedRange() which has its own warning. */
 {
@@ -2452,44 +2472,44 @@
 
 static char *hFreezeDbConversion(char *database, char *freeze)
 /* Find freeze given database or vice versa.  Pass in NULL
  * for parameter that is unknown and it will be returned
  * as a result.  This result can be freeMem'd when done. */
 {
 if ((database != NULL) && trackHubDatabase(database))
     {
     return trackHubAssemblyField(database, "description");
     }
 
 struct sqlConnection *conn = hConnectCentral();
 struct sqlResult *sr;
 char **row;
 char *ret = NULL;
-struct dyString *dy = newDyString(128);
+struct dyString *dy = dyStringNew(128);
 
 if (database != NULL)
     sqlDyStringPrintf(dy, "select description from %s where name = '%s'", dbDbTable(), database);
 else if (freeze != NULL)
     sqlDyStringPrintf(dy, "select name from %s where description = '%s'", dbDbTable(), freeze);
 else
     internalErr();
 sr = sqlGetResult(conn, dy->string);
 if ((row = sqlNextRow(sr)) != NULL)
     ret = cloneString(row[0]);
 sqlFreeResult(&sr);
 hDisconnectCentral(&conn);
-freeDyString(&dy);
+dyStringFree(&dy);
 return ret;
 }
 
 
 char *hFreezeFromDb(char *database)
 /* return the freeze for the database version.
    For example: "hg6" returns "Dec 12, 2000". If database
    not recognized returns NULL */
 {
 return hFreezeDbConversion(database, NULL);
 }
 
 char *hDbFromFreeze(char *freeze)
 /* Return database version from freeze name. */
 {
@@ -2790,31 +2810,31 @@
 return db;
 }
 
 struct dbDb *hDbDbListMaybeCheck(boolean doCheck)
 /* Return list of databases in dbDb.  If doCheck, check database existence.
  * The list includes the name, description, and where to
  * find the nib-formatted DNA files. Free this with dbDbFree. */
 {
 struct sqlConnection *conn = hConnectCentral();
 struct sqlResult *sr;
 char **row;
 struct dbDb *dbList = NULL, *db;
 struct hash *hash = sqlHashOfDatabases();
 
 char query[1024];
-safef(query, sizeof query,  NOSQLINJ "select * from %s order by orderKey,name desc", dbDbTable());
+sqlSafef(query, sizeof query,  "select * from %s order by orderKey,name desc", dbDbTable());
 sr = sqlGetResult(conn, query);
 while ((row = sqlNextRow(sr)) != NULL)
     {
     db = dbDbLoad(row);
     boolean isGenarkHub = sameOk(db->nibPath, "genark");
     if (!doCheck || (isGenarkHub || hashLookup(hash, db->name)))
         {
 	slAddHead(&dbList, db);
 	}
     else
         dbDbFree(&db);
     }
 sqlFreeResult(&sr);
 hashFree(&hash);
 hDisconnectCentral(&conn);
@@ -3572,82 +3592,82 @@
  * and for each chromosome (which is assumed to be less than
  * 512M.)  A range goes into the smallest bin it will fit in. */
 {
 return binFromRange(start, end);
 }
 
 static void hAddBinToQueryStandard(char *binField, int start, int end,
 	struct dyString *query, boolean selfContained)
 /* Add clause that will restrict to relevant bins to query. */
 {
 int bFirstShift = binFirstShift(), bNextShift = binNextShift();
 int startBin = (start>>bFirstShift), endBin = ((end-1)>>bFirstShift);
 int i, levels = binLevels();
 
 if (selfContained)
-    dyStringAppend(query, "(");
+    sqlDyStringPrintf(query, "(");
 for (i=0; i<levels; ++i)
     {
     int offset = binOffset(i);
     if (i != 0)
-        dyStringAppend(query, " or ");
+        sqlDyStringPrintf(query, " or ");
     if (startBin == endBin)
-        dyStringPrintf(query, "%s=%u", binField, startBin + offset);
+        sqlDyStringPrintf(query, "%s=%u", binField, startBin + offset);
     else
-        dyStringPrintf(query, "%s>=%u and %s<=%u",
+        sqlDyStringPrintf(query, "%s>=%u and %s<=%u",
 		binField, startBin + offset, binField, endBin + offset);
     startBin >>= bNextShift;
     endBin >>= bNextShift;
     }
 if (selfContained)
     {
-    dyStringPrintf(query, " or %s=%u )", binField, _binOffsetOldToExtended);
-    dyStringAppend(query, " and ");
+    sqlDyStringPrintf(query, " or %s=%u )", binField, _binOffsetOldToExtended);
+    sqlDyStringPrintf(query, " and ");
     }
 }
 
 static void hAddBinToQueryExtended(char *binField, int start, int end,
 	struct dyString *query)
 /* Add clause that will restrict to relevant bins to query. */
 {
 int bFirstShift = binFirstShift(), bNextShift = binNextShift();
 int startBin = (start>>bFirstShift), endBin = ((end-1)>>bFirstShift);
 int i, levels = binLevelsExtended();
 
-dyStringAppend(query, "(");
+sqlDyStringPrintf(query, "(");
 
 if (start < BINRANGE_MAXEND_512M)
     {
     hAddBinToQueryStandard(binField, start, BINRANGE_MAXEND_512M, query, FALSE);
-    dyStringAppend(query, " or ");
+    sqlDyStringPrintf(query, " or ");
     }
 
 for (i=0; i<levels; ++i)
     {
     int offset = binOffsetExtended(i);
     if (i != 0)
-	dyStringAppend(query, " or ");
+	sqlDyStringPrintf(query, " or ");
     if (startBin == endBin)
         sqlDyStringPrintf(query, "%s=%u", binField, startBin + offset);
     else
         sqlDyStringPrintf(query, "%s>=%u and %s<=%u",
 		binField, startBin + offset, binField, endBin + offset);
     startBin >>= bNextShift;
     endBin >>= bNextShift;
     }
-dyStringAppend(query, ")");
-dyStringAppend(query, " and ");
+sqlDyStringPrintf(query, ")");
+sqlDyStringPrintf(query, " and ");
 }
 
 void hAddBinToQueryGeneral(char *binField, int start, int end,
 	struct dyString *query)
 /* Add clause that will restrict to relevant bins to query. */
 {
 if (end <= BINRANGE_MAXEND_512M)
     hAddBinToQueryStandard(binField, start, end, query, TRUE);
 else
     hAddBinToQueryExtended(binField, start, end, query);
 }
 
 void hAddBinToQuery(int start, int end, struct dyString *query)
 /* Add clause that will restrict to relevant bins to query. */
 {
@@ -3656,45 +3676,44 @@
 
 struct sqlResult *hExtendedRangeQuery(
 	struct sqlConnection *conn,  /* Open SQL connection. */
 	char *rootTable, 	     /* Table (not including any chrN_) */
 	char *chrom, int start, int end,  /* Range. */
 	char *extraWhere,            /* Extra things to add to where clause. */
 	boolean order, 	   /* If true order by start position (can be slow). */
 	char *fields,      /* If non-NULL comma separated field list. */
 	int *retRowOffset) /* Returns offset past bin field. */
 /* Range query with lots of options. */
 {
 char *db = sqlGetDatabase(conn);
 /* call hFindTableInfoWithConn() to support tracks may from different hosts */
 struct hTableInfo *hti = hFindTableInfoWithConn(conn, chrom, rootTable);
 struct sqlResult *sr = NULL;
-struct dyString *query = newDyString(1024);
+struct dyString *query = dyStringNew(1024);
 char *table = NULL;
 char fullTable[HDB_MAX_TABLE_STRING];
 int rowOffset = 0;
 
 if (fields == NULL) fields = "*";
 if (hti == NULL)
     {
     warn("table %s doesn't exist in %s database, or hFindTableInfoDb failed", rootTable, db);
     }
 else
     {
-    if (!sameString(fields,"*"))
-	sqlCkIl(fields);
-    sqlDyStringPrintf(query, "select %-s from ", fields);
+    sqlCkIl(fieldsSafe,fields)
+    sqlDyStringPrintf(query, "select %-s from ", fieldsSafe);
     if (hti->isSplit)
 	{
 	safef(fullTable, sizeof(fullTable), "%s_%s", chrom, rootTable);
 	if (!hTableExists(db, fullTable))
 	     warn("%s doesn't exist", fullTable);
 	else
 	    {
 	    table = fullTable;
 	    sqlDyStringPrintf(query, "%s where ", table);
 	    }
 	}
     else
         {
 	table = hti->rootName;
 	sqlDyStringPrintf(query, "%s where %s='%s' and ",
@@ -3703,40 +3722,40 @@
     }
 if (table != NULL)
     {
     if (hti->hasBin)
         {
 	hAddBinToQuery(start, end, query);
 	rowOffset = 1;
 	}
     if (start == end) // e.g. size = 0 SNP insertion site
     	sqlDyStringPrintf(query, "%s=%u and %s=%u", hti->startField, end, hti->endField, start);
     else
     	sqlDyStringPrintf(query, "%s<%u and %s>%u", hti->startField, end, hti->endField, start);
     if (extraWhere)
         {
         /* allow more flexible additions to where clause */
-        if (!startsWith("order", extraWhere) &&
-            !startsWith("limit", extraWhere))
-                dyStringAppend(query, " and ");
-        dyStringPrintf(query, " %s", extraWhere);
+        if (!startsWith(NOSQLINJ "order", extraWhere) &&
+            !startsWith(NOSQLINJ "limit", extraWhere))
+                sqlDyStringPrintf(query, " and ");
+        sqlDyStringPrintf(query, " %-s", extraWhere);
         }
     if (order)
         sqlDyStringPrintf(query, " order by %s", hti->startField);
     sr = sqlGetResult(conn, query->string);
     }
-freeDyString(&query);
+dyStringFree(&query);
 if (retRowOffset != NULL)
     *retRowOffset = rowOffset;
 return sr;
 }
 
 struct sqlResult *hRangeQuery(struct sqlConnection *conn,
 	char *rootTable, char *chrom,
 	int start, int end, char *extraWhere, int *retRowOffset)
 /* Construct and make a query to tables that may be split and/or
  * binned. */
 {
 return hExtendedRangeQuery(conn, rootTable, chrom, start, end,
 	extraWhere, FALSE, NULL, retRowOffset);
 }
 
@@ -3752,63 +3771,61 @@
 
 struct sqlResult *hExtendedChromQuery(
 	struct sqlConnection *conn,  /* Open SQL connection. */
 	char *rootTable, 	     /* Table (not including any chrN_) */
 	char *chrom,  		     /* Chromosome. */
 	char *extraWhere,            /* Extra things to add to where clause. */
 	boolean order, 	   /* If true order by start position (can be slow). */
 	char *fields,      /* If non-NULL comma separated field list. */
 	int *retRowOffset) /* Returns offset past bin field. */
 /* Chromosome query fields for tables that may be split and/or binned,
  * with lots of options. */
 {
 char *db = sqlGetDatabase(conn);
 struct hTableInfo *hti = hFindTableInfo(db, chrom, rootTable);
 struct sqlResult *sr = NULL;
-struct dyString *query = newDyString(1024);
+struct dyString *query = dyStringNew(1024);
 int rowOffset = 0;
 
 if (fields == NULL) fields = "*";
 if (hti == NULL)
     {
     warn("table %s doesn't exist", rootTable);
     }
 else
     {
     rowOffset = hti->hasBin;
     if (hti->isSplit)
 	{
-	if (!sameString(fields,"*"))
-	    sqlCkIl(fields);
-        sqlDyStringPrintf(query, "select %-s from %s_%s", fields, chrom, rootTable);
+        sqlCkIl(fieldsSafe,fields)
+        sqlDyStringPrintf(query, "select %-s from %s_%s", fieldsSafe, chrom, rootTable);
 	if (extraWhere != NULL)
-	    dyStringPrintf(query, " where %s", extraWhere);
+	    sqlDyStringPrintf(query, " where %-s", extraWhere);
 	}
     else
 	{
-	if (!sameString(fields,"*"))
-	    sqlCkIl(fields);
+        sqlCkIl(fieldsSafe,fields)
         sqlDyStringPrintf(query, "select %-s from %s where %s='%s'",
-		fields, hti->rootName, hti->chromField, chrom);
+		fieldsSafe, hti->rootName, hti->chromField, chrom);
 	if (extraWhere != NULL)
-	    dyStringPrintf(query, " and (%s)", extraWhere);
+	    sqlDyStringPrintf(query, " and (%-s)", extraWhere);
 	}
     if (order)
         sqlDyStringPrintf(query, " order by %s", hti->startField);
     sr = sqlGetResult(conn, query->string);
     }
-freeDyString(&query);
+dyStringFree(&query);
 if (retRowOffset != NULL)
     *retRowOffset = rowOffset;
 return sr;
 }
 
 struct sqlResult *hChromQuery(struct sqlConnection *conn,
 	char *rootTable, char *chrom,
 	char *extraWhere, int *retRowOffset)
 /* Construct and make a query across whole chromosome to tables
  * that may be split and/or * binned. */
 {
 return hExtendedChromQuery(conn, rootTable, chrom, extraWhere,
 	FALSE, NULL, retRowOffset);
 }
 
@@ -4233,31 +4250,31 @@
 					    char *where)
 /* Load trackDb object(s). Nothing done for composite tracks here. */
 {
 return loadTrackDb(sqlGetDatabase(conn), where);
 }
 
 static struct trackDb *loadTrackDbForTrack(struct sqlConnection *conn,
 					   char *track)
 /* Load trackDb object for a track. this is common code for two external
  * functions. Handle composite tracks and subtrack inheritance here.
  */
 {
 struct trackDb *trackTdb = NULL;
 char where[256];
 
-sqlSafefFrag(where, sizeof(where), "tableName = '%s'", track);
+sqlSafef(where, sizeof(where), "tableName = '%s'", track);
 trackTdb = loadAndLookupTrackDb(conn, where);
 if (!trackTdb)
     return NULL;
 return trackTdb;
 }
 
 struct trackDb *tdbForTrack(char *db, char *track,struct trackDb **tdbList)
 /* Load trackDb object for a track. If track is composite, its subtracks
  * will also be loaded and inheritance will be handled; if track is a
  * subtrack then inheritance will be handled.  (Unless a subtrack has
  * "noInherit on"...) This will die if the current database does not have
  * a trackDb, but will return NULL if track is not found.
  * MAY pass in prepopulated trackDb list, or may receive the trackDb list as an inout. */
 {
 /* Get track list .*/
@@ -4673,41 +4690,42 @@
 }
 
 struct dbDb *hGetIndexedDatabasesForClade(char *db)
 /* Get list of active databases in db's clade.
  * Dispose of this with dbDbFreeList. */
 {
 return hGetIndexedDbsMaybeClade(db);
 }
 
 struct slPair *hGetCladeOptions()
 /* Return a list of slPairs, each containing clade menu value (hgcentral.clade.name, e.g. 'mammal')
  * and clade menu label (hgcentral.clade.label, e.g. 'Mammal'),
  * useful for constructing a clade menu. */
 {
 // get only the clades that have actual active genomes
-char *query = NOSQLINJ ""
+char query[4096];
+sqlSafef(query, sizeof query, 
     "SELECT DISTINCT(c.name), c.label "
-    // mysql 5.7: SELECT list w/DISTINCT must include all fields in ORDER BY list (#18626)
     ", c.priority "
     "FROM %s c, %s g, %s d "
     "WHERE c.name=g.clade AND d.organism=g.genome AND d.active=1 "
-    "ORDER BY c.priority";
-char queryBuf[4096];
-safef(queryBuf, sizeof queryBuf, query, cladeTable(),  genomeCladeTable(), dbDbTable());
+    "ORDER BY c.priority"
+    , cladeTable(),  genomeCladeTable(), dbDbTable());
+// mysql 5.7: SELECT list w/DISTINCT must include all fields in ORDER BY list (#18626)
+
 struct sqlConnection *conn = hConnectCentral();
-struct slPair *nativeClades = sqlQuickPairList(conn, queryBuf);
+struct slPair *nativeClades = sqlQuickPairList(conn, query);
 hDisconnectCentral(&conn);
 struct slPair *trackHubClades = trackHubGetCladeLabels();
 return slCat(nativeClades, trackHubClades);
 }
 
 struct slPair *hGetGenomeOptionsForClade(char *clade)
 /* Return a list of slPairs, each containing genome menu value and menu label,
  * useful for constructing a genome menu for the given clade. */
 {
 struct slPair *pairList = NULL;
 if (isHubTrack(clade))
     {
     struct dbDb *hubDbDbList = trackHubGetDbDbs(clade), *dbDb;
     for (dbDb = hubDbDbList;  dbDb != NULL;  dbDb = dbDb->next)
 	slAddHead(&pairList, slPairNew(dbDb->genome, cloneString(dbDb->genome)));
@@ -4931,39 +4949,40 @@
 /* to the oldest assemblies in the dropdown menu for toDbs */
 slSort(&liftOverDbList, hDbDbCmpOrderKey);
 return liftOverDbList;
 }
 
 struct dbDb *hGetBlatIndexedDatabases()
 /* Get list of databases for which there is a BLAT index.
  * Dispose of this with dbDbFreeList. */
 {
 struct hash *hash=newHash(5);
 struct sqlConnection *conn = hConnectCentral();
 struct sqlResult *sr;
 char **row;
 struct dbDb *dbList = NULL, *db;
 
+char query[1024];
 /* Get hash of active blat servers. */
-sr = sqlGetResult(conn, NOSQLINJ "select db from blatServers");
+sqlSafef(query,  sizeof query, "select db from blatServers");
+sr = sqlGetResult(conn, query);
 while ((row = sqlNextRow(sr)) != NULL)
     hashAdd(hash, row[0], NULL);
 sqlFreeResult(&sr);
 
 /* Scan through dbDb table, keeping ones that are indexed. */
-char query[1024];
-safef(query,  sizeof query, NOSQLINJ "select * from %s order by orderKey,name desc", dbDbTable());
+sqlSafef(query,  sizeof query, "select * from %s order by orderKey,name desc", dbDbTable());
 sr = sqlGetResult(conn, query);
 while ((row = sqlNextRow(sr)) != NULL)
     {
     db = dbDbLoad(row);
     if (hashLookup(hash, db->name))
         {
 	slAddHead(&dbList, db);
 	}
     else
         dbDbFree(&db);
     }
 sqlFreeResult(&sr);
 hDisconnectCentral(&conn);
 hashFree(&hash);
 slReverse(&dbList);
@@ -5031,77 +5050,82 @@
 return &st;
 }
 
 char *sqlGetField(char *db, char *tblName, char *fldName,
   	          char *condition)
 /* Return a single field from the database, table name, field name, and a
    condition string */
 {
 struct sqlConnection *conn = hAllocConn(db);
 char query[256];
 struct sqlResult *sr;
 char **row;
 char *answer;
 
 answer = NULL;
+sqlCkIl(tblNameSafe,tblName)
 sqlSafef(query, sizeof(query), "select %s from %-s  where %-s;",
-      fldName,  sqlCheckIdentifiersList(tblName), condition);  // note some callers pass an entire tables list with aliases in tblName
+      fldName,  tblNameSafe, condition);  // note some callers pass an entire tables list with aliases in tblName
 sr  = sqlGetResult(conn, query);
 row = sqlNextRow(sr);
 
 if (row != NULL)
     {
     answer = cloneString(row[0]);
     }
 
 sqlFreeResult(&sr);
 hFreeConn(&conn);
 return answer;
 }
 
 struct hash *hChromSizeHash(char *db)
 /* Get hash of chromosome sizes for database.  Just hashFree it when done. */
 {
 struct sqlConnection *conn = sqlConnect(db);
 struct sqlResult *sr;
 char **row;
 struct hash *hash = newHash(0);
-sr = sqlGetResult(conn, NOSQLINJ "select chrom,size from chromInfo");
+char query[1024];
+sqlSafef(query, sizeof query, "select chrom,size from chromInfo");
+sr = sqlGetResult(conn, query);
 while ((row = sqlNextRow(sr)) != NULL)
     hashAddInt(hash, row[0], sqlUnsigned(row[1]));
 sqlFreeResult(&sr);
 sqlDisconnect(&conn);
 return hash;
 }
 
 struct hash *hChromSizeHashFromFile(char *fileName)
 /* Get hash of chromosome sizes from 2- or 3-column chrom.sizes file. hashFree when done. */
 {
 struct hash *chromHash = hashNew(0);
 struct lineFile *lf = lineFileOpen(fileName, TRUE);
 char *row[2];
 while (lineFileRow(lf, row))
     hashAddInt(chromHash, row[0], sqlUnsigned(row[1]));
 lineFileClose(&lf);
 return chromHash;
 }
 
 struct slName *hChromList(char *db)
 /* Get the list of chrom names from the database's chromInfo table. */
 {
 struct sqlConnection *conn = hAllocConn(db);
-struct slName *list = sqlQuickList(conn, NOSQLINJ "select chrom from chromInfo");
+char query[1024];
+sqlSafef(query, sizeof query, "select chrom from chromInfo");
+struct slName *list = sqlQuickList(conn, query);
 hFreeConn(&conn);
 return list;
 }
 
 char *hgDirForOrg(char *org)
 /* Make directory name from organism name - getting
  * rid of dots and spaces. */
 {
 org = cloneString(org);
 stripChar(org, '.');
 subChar(org, ' ', '_');
 return org;
 }
 
 struct hash *hgReadRa(char *genome, char *database, char *rootDir,
@@ -5813,32 +5837,33 @@
 /* Return the RefSeq NC_000... accession for chrom if we can find it, else just chrom.
  * db must never change. */
 {
 static char *firstDb = NULL;
 static struct hash *accHash = NULL;
 static boolean checkExistence = TRUE;
 if (firstDb && !sameString(firstDb, db))
     errAbort("hRefSeqAccForChrom: only works for one db.  %s was passed in earlier, now %s.",
              firstDb, db);
 char *seqAcc = NULL;
 if (checkExistence && !trackHubDatabase(db) && hTableExists(db, "chromAlias"))
     // Will there be a chromAlias for hubs someday??
     {
     firstDb = db;
     struct sqlConnection *conn = hAllocConn(db);
-    accHash = sqlQuickHash(conn,
-                           NOSQLINJ "select chrom, alias from chromAlias where source = 'refseq'");
+    char query[1024];
+    sqlSafef(query, sizeof query, "select chrom, alias from chromAlias where source = 'refseq'");
+    accHash = sqlQuickHash(conn, query);
     if (hashNumEntries(accHash) == 0)
         // No RefSeq accessions -- make accHash NULL
         hashFree(&accHash);
     hFreeConn(&conn);
     checkExistence = FALSE;
     }
 if (accHash)
     seqAcc = cloneString(hashFindVal(accHash, chrom));
 if (seqAcc == NULL)
     seqAcc = cloneString(chrom);
 return seqAcc;
 }
 
 char *abbreviateRefSeqSummary(char *summary) 
 /* strip off the uninformative parts from the RefSeq Summary text: the repetitive note