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/inc/jksql.h src/hg/inc/jksql.h
index 8608ffc..df12032 100644
--- src/hg/inc/jksql.h
+++ src/hg/inc/jksql.h
@@ -620,33 +620,37 @@
 /* dump internal info about SQL configuration for debugging purposes */
 
 void sqlPrintStats(FILE *fh);
 /* print statistic about the number of connections and other options done by
  * this process. */
 
 struct sqlResult *sqlStoreResult(struct sqlConnection *sc, char *query);
 /* Returns NULL if result was empty.  Otherwise returns a structure
  * that you can do sqlRow() on.  Same interface as sqlGetResult,
  * but internally this keeps the entire result in memory. */
 
 
 
 /* --------- input checks to prevent sql injection --------------------------------------- */
 
-#define sqlCkIl sqlCheckIdentifiersList
-char *sqlCheckIdentifiersList(char *identifiers);
-/* Check that only valid identifier characters are used in a comma-separated list */
+//#define sqlCkIl sqlCheckIdentifiersList
+#define sqlCkIl(fieldsSafe,fields) char fieldsSafe[strlen(fields)+9+1]; \
+   sqlCheckIdentifiersList(fieldsSafe, sizeof fieldsSafe, fields);
+
+void sqlCheckIdentifiersList(char* buffer, int bufSize, char *identifiers);
+/* Check that only valid identifier characters are used in a comma-separated list.
+ * Save safe-marked identifiers list to buf. */
 
 #define sqlCkId sqlCheckIdentifier
 char *sqlCheckIdentifier(char *identifier);
 /* Check that only valid identifier characters are used */
 
 
 // =============================
 
 int vaSqlSafefNoAbort(char* buffer, int bufSize, boolean newString, char *format, va_list args);
 /* VarArgs Format string to buffer, vsprintf style, only with buffer overflow
  * checking.  The resulting string is always terminated with zero byte.
  * Scans string parameters for illegal sql chars. 
  * Automatically escapes quoted string values.
  * This function should be efficient on statements with many strings to be escaped. */
 
@@ -656,104 +660,54 @@
  * Scans unquoted string parameters for illegal literal sql chars.
  * Escapes quoted string parameters. 
  * NOSLQINJ tag is added to beginning. */
 
 int sqlSafef(char* buffer, int bufSize, char *format, ...)
 /* Format string to buffer, vsprintf style, only with buffer overflow
  * checking.  The resulting string is always terminated with zero byte. 
  * Scans unquoted string parameters for illegal literal sql chars.
  * Escapes quoted string parameters. 
  * NOSLQINJ tag is added to beginning. */
 #ifdef __GNUC__
 __attribute__((format(printf, 3, 4)))
 #endif
 ;
 
-
-int vaSqlSafefFrag(char* buffer, int bufSize, char *format, va_list args);
-/* VarArgs Format string to buffer, vsprintf style, only with buffer overflow
- * checking.  The resulting string is always terminated with zero byte.
- * Scans unquoted string parameters for illegal literal sql chars.
- * Escapes quoted string parameters. 
- * NOSLQINJ tag is NOT added to beginning since it is assumed to be just a fragment of
- * the entire sql string. */
-
-int sqlSafefFrag(char* buffer, int bufSize, char *format, ...)
-/* Format string to buffer, vsprintf style, only with buffer overflow
- * checking.  The resulting string is always terminated with zero byte.
- * Scans unquoted string parameters for illegal literal sql chars.
- * Escapes quoted string parameters. 
- * NOSLQINJ tag is NOT added to beginning since it is assumed to be just a fragment of
- * the entire sql string. */
-#ifdef __GNUC__
-__attribute__((format(printf, 3, 4)))
-#endif
-;
-
-
-int sqlSafefAppend(char* buffer, int bufSize, char *format, ...)
-/* Append formatted string to buffer, vsprintf style, only with buffer overflow
- * checking.  The resulting string is always terminated with zero byte.
- * Scans unquoted string parameters for illegal literal sql chars.
- * Escapes quoted string parameters. 
- * NOSLQINJ tag is NOT added to beginning since it is assumed to be appended to
- * a properly created sql string. */
-#ifdef __GNUC__
-__attribute__((format(printf, 3, 4)))
-#endif
-;
-
-
-void vaSqlDyStringPrintfExt(struct dyString *ds, boolean isFrag, char *format, va_list args);
+void vaSqlDyStringPrintfExt(struct dyString *ds, char *format, va_list args);
 /* VarArgs Printf to end of dyString after scanning string parameters for illegal sql chars.
  * Strings inside quotes are automatically escaped.  
- * NOSLQINJ tag is added to beginning if it is a new empty string and isFrag is FALSE.
+ * NOSLQINJ tag is added to beginning if it is a new empty string.
  * Appends to existing string. */
 
 void vaSqlDyStringPrintf(struct dyString *ds, char *format, va_list args);
 /* Printf to end of dyString after scanning string parameters for illegal sql chars.
  * Strings inside quotes are automatically escaped.  
  * NOSLQINJ tag is added to beginning if it is a new empty string.
  * Appends to existing string. */
 
 void sqlDyStringPrintf(struct dyString *ds, char *format, ...)
 /* Printf to end of dyString after scanning string parameters for illegal sql chars.
  * Strings inside quotes are automatically escaped.  
  * NOSLQINJ tag is added to beginning if it is a new empty string.
  * Appends to existing string. */
 #ifdef __GNUC__
 __attribute__((format(printf, 2, 3)))
 #endif
 ;
 
-void vaSqlDyStringPrintfFrag(struct dyString *ds, char *format, va_list args);
-/* VarArgs Printf to end of dyString after scanning string parameters for illegal sql chars.
- * Strings inside quotes are automatically escaped.
- * NOSLQINJ tag is NOT added to beginning since it is assumed to be just a fragment of
- * the entire sql string. Appends to existing string. */
-
-void sqlDyStringPrintfFrag(struct dyString *ds, char *format, ...)
-/* Printf to end of dyString after scanning string parameters for illegal sql chars.
- * Strings inside quotes are automatically escaped.
- * NOSLQINJ tag is NOT added to beginning since it is assumed to be just a fragment of
- * the entire sql string. Appends to existing string. */
-#ifdef __GNUC__
-__attribute__((format(printf, 2, 3)))
-#endif
-;
-
 #define NOSQLINJ "NOSQLINJ "
+#define NOSQLINJ_SIZE 9
 
 struct dyString *sqlDyStringCreate(char *format, ...)
 /* Create a dyString with a printf style initial content
  * Adds the NOSQLINJ prefix. */
 #ifdef __GNUC__
 __attribute__((format(printf, 1, 2)))
 #endif
 ;
 
 void sqlDyStringPrintIdList(struct dyString *ds, char *fields);
 /* Append a comma-separated list of field identifiers. Aborts if invalid characters in list. */
 
 void sqlDyStringPrintValuesList(struct dyString *ds, struct slName *values);
 /* Append a comma-separated, quoted and escaped list of values. */