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/hgTracks/wigTrack.c src/hg/hgTracks/wigTrack.c
index 86c6497..5c09ac3 100644
--- src/hg/hgTracks/wigTrack.c
+++ src/hg/hgTracks/wigTrack.c
@@ -399,42 +399,39 @@
  *
  *	With 1K zoom Spans available, no more than approximately 1024
  *	rows will need to be loaded at any one time.
  */
 {
 struct sqlConnection *conn = NULL ;
 
 // if this is a custom track we don't need an SQL connection to the database
 if (!isCustomTrack(tg->table))
     conn = hAllocConn(database);
 struct sqlResult *sr;
 char **row;
 int rowOffset;
 struct wiggle wiggle;
 struct wigItem *wiList = NULL;
-char *whereNULL = NULL;
 int itemsLoaded = 0;
 struct hash *spans = NULL;	/* Spans encountered during load */
 /*	Check our scale from the global variables that exist in
  *	hgTracks.c - This can give us a guide about which rows to load.
  *	If the scale is more than 1000 bases per pixel, we can try loading
  *	only those rows with Span >= 1000 to see if an appropriate zoom
  *	level exists.
  */
 int basesPerPixel = (int)((double)(winEnd - winStart)/(double)insideWidth);
-char *span1K = "Span >= 1000 limit 1";
-char *spanOver1K = "Span >= 1000";
 char whereSpan[SMALLBUF];
 int spanMinimum = 1;
 char *dbTableName = NULL;
 struct trackDb *tdb = NULL;
 int loadStart = winStart, loadEnd = winEnd;
 
 #ifndef GBROWSE
 struct customTrack *ct = NULL;
 /*	custom tracks have different database	*/
 if (isCustomTrack(tg->table) && tg->customPt)
     {
     hFreeConn(&conn);
     conn = hAllocConn(CUSTOM_TRASH);
     ct = tg->customPt;
     dbTableName = ct->dbTableName;
@@ -458,75 +455,77 @@
 /*	Allocate trackSpans one time only	*/
 if (! trackSpans)
     trackSpans = newHash(0);
 
 /*	find the minimum span to see if there are actually any data
  *	points in this area at that span.  If there are not, then there
  *	is no data here even if a zoomed view covers this section.
  *	protect against less than 1 with the max(1,minSpan());
  *	This business will fix the problem mentioned in RT #1186
  */
 
 spanMinimum = max(1,
 	minSpan(conn, dbTableName, chromName, winStart, winEnd, cart, tdb));
 
 itemsLoaded = 0;
-sqlSafefFrag(whereSpan, sizeof(whereSpan), "span=%d limit 1", spanMinimum);
+sqlSafef(whereSpan, sizeof(whereSpan), "span=%d limit 1", spanMinimum);
 
 sr = hRangeQuery(conn, dbTableName, chromName, loadStart, loadEnd,
     whereSpan, &rowOffset);
 
 while ((row = sqlNextRow(sr)) != NULL)
     ++itemsLoaded;
 sqlFreeResult(&sr);
 
 /*	if nothing here, bail out	*/
 if (itemsLoaded < 1)
     {
     tg->items = (struct wigItem *)NULL;
     hFreeConn(&conn);
     return;
     }
 
 itemsLoaded = 0;
 if (basesPerPixel >= 1000)
     {
+    sqlSafef(whereSpan, sizeof(whereSpan), "Span >= 1000 limit 1");
     sr = hRangeQuery(conn, dbTableName, chromName, loadStart, loadEnd,
-	span1K, &rowOffset);
+	whereSpan, &rowOffset);
     while ((row = sqlNextRow(sr)) != NULL)
 	    ++itemsLoaded;
     sqlFreeResult(&sr);
     }
 /*	If that worked, excellent, then we have at least another zoom level
  *	So, for our actual load, use spanOver1K to fetch not only the 1K
  *	zooms, but potentially others that may be useful.  This will
  *	save us a huge amount in loaded rows.  On a 250 Mbase chromosome
  *	there would be 256,000 rows at the 1 base level and only
  *	256 rows at the 1K zoom level.  Otherwise, we go back to the
  *	regular query which will give us all rows.
  */
 /* JK - Can't we figure out here one, and only one span to load?  This
  * would simplify drawing logic. */
 if (itemsLoaded)
     {
+    sqlSafef(whereSpan, sizeof(whereSpan), "Span >= 1000");
     sr = hRangeQuery(conn, dbTableName, chromName, loadStart, loadEnd,
-	     spanOver1K, &rowOffset);
+	     whereSpan, &rowOffset);
     }
 else
     {
     sr = hRangeQuery(conn, dbTableName, chromName, loadStart, loadEnd,
-	whereNULL, &rowOffset);
+	NULL, &rowOffset);
     }
 
 /*	Allocate trackSpans one time only	*/
 if (! trackSpans)
     trackSpans = newHash(4);
 /*	Each instance of this LoadItems will create a new spans hash
  *	It will be the value included in the trackSpans hash
  */
 spans = newHash(4);
 /*	Each row read will be turned into an instance of a wigItem
  *	A growing list of wigItems will be the items list to return
  */
 itemsLoaded = 0;
 while ((row = sqlNextRow(sr)) != NULL)
     {