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/visiGene/visiGeneLoad/visiGeneLoad.c src/hg/visiGene/visiGeneLoad/visiGeneLoad.c index 3c18609..97e2f4e 100644 --- src/hg/visiGene/visiGeneLoad/visiGeneLoad.c +++ src/hg/visiGene/visiGeneLoad/visiGeneLoad.c @@ -157,62 +157,62 @@ int findOrAddSubmissionSource(struct sqlConnection *conn, char *name, char *acknowledgement, char *setUrl, char *itemUrl, char *abUrl) /* Return submissionSource table id. If necessary creating row in * submissionSource table. */ { char query[256]; sqlSafef(query, sizeof(query), "select id from submissionSource where name = \"%s\"", name); int id = sqlQuickNum(conn, query); if (id == 0) { struct dyString *dy = dyStringNew(0); sqlDyStringPrintf(dy, "insert into submissionSource values(default, "); - dyStringPrintf(dy, "\"%s\", ", name); - dyStringPrintf(dy, "\"%s\", ", acknowledgement); - dyStringPrintf(dy, "\"%s\", ", setUrl); - dyStringPrintf(dy, "\"%s\",", itemUrl); - dyStringPrintf(dy, "\"%s\")", abUrl); + sqlDyStringPrintf(dy, "\"%s\", ", name); + sqlDyStringPrintf(dy, "\"%s\", ", acknowledgement); + sqlDyStringPrintf(dy, "\"%s\", ", setUrl); + sqlDyStringPrintf(dy, "\"%s\",", itemUrl); + sqlDyStringPrintf(dy, "\"%s\")", abUrl); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); dyStringFree(&dy); id = sqlLastAutoId(conn); } return id; } int doJournal(struct sqlConnection *conn, char *name, char *url) /* Update journal table if need be. Return journal ID. */ { if (name[0] == 0) return 0; else { struct dyString *dy = dyStringNew(0); int id = 0; sqlDyStringPrintf(dy, "select id from journal where name = '%s'", name); id = sqlQuickNum(conn, dy->string); if (id == 0) { dyStringClear(dy); sqlDyStringPrintf(dy, "insert into journal set"); - dyStringPrintf(dy, " id=default,\n"); - dyStringPrintf(dy, " name=\"%s\",\n", name); - dyStringPrintf(dy, " url=\"%s\"\n", url); + sqlDyStringPrintf(dy, " id=default,\n"); + sqlDyStringPrintf(dy, " name=\"%s\",\n", name); + sqlDyStringPrintf(dy, " url=\"%s\"\n", url); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); id = sqlLastAutoId(conn); } dyStringFree(&dy); return id; } } int createSubmissionId(struct sqlConnection *conn, char *name, char *contributors, char *publication, char *pubUrl, int submissionSource, char *journal, char *journalUrl, int copyright, @@ -714,31 +714,31 @@ sqlUpdate(conn, query->string); dyStringFree(&query); id = sqlLastAutoId(conn); hashAddInt(captionIdCache, captionExtId, id); return id; } } int doImageFile(struct lineFile *lf, struct sqlConnection *conn, char *fileName, int fullDir, int thumbDir, int submissionSetId, char *submitId, char *priority, int captionId, int imageWidth, int imageHeight) /* Update image file record if necessary and return image file ID. */ { int imageFileId = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); sqlDyStringPrintf(dy, "select id from imageFile where "); sqlDyStringPrintf(dy, "fileName='%s' and fullLocation=%d", fileName, fullDir); imageFileId = sqlQuickNum(conn, dy->string); if (imageFileId == 0) { dyStringClear(dy); sqlDyStringPrintf(dy, "insert into imageFile set"); sqlDyStringPrintf(dy, " id = default,\n"); sqlDyStringPrintf(dy, " fileName = '%s',\n", fileName); sqlDyStringPrintf(dy, " priority = '%s',\n", priority); sqlDyStringPrintf(dy, " fullLocation = %d,\n", fullDir); sqlDyStringPrintf(dy, " thumbLocation = %d,\n", thumbDir); sqlDyStringPrintf(dy, " submissionSet = %d,\n", submissionSetId); @@ -747,60 +747,60 @@ sqlDyStringPrintf(dy, " imageWidth = %d,\n", imageWidth); sqlDyStringPrintf(dy, " imageHeight = %d\n", imageHeight); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); imageFileId = sqlLastAutoId(conn); } dyStringFree(&dy); return imageFileId; } int doStrain(struct lineFile *lf, struct sqlConnection *conn, char *taxon, char *strain) /* Return strain id, creating a new table entry if necessary. */ { int id = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); sqlDyStringPrintf(dy, "select id from strain where "); sqlDyStringPrintf(dy, "taxon=%s and name='%s'", taxon, strain); id = sqlQuickNum(conn, dy->string); if (id == 0) { dyStringClear(dy); sqlDyStringPrintf(dy, "insert into strain set"); sqlDyStringPrintf(dy, " id = default,\n"); sqlDyStringPrintf(dy, " taxon = '%s',\n", taxon); sqlDyStringPrintf(dy, " name = '%s'", strain); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); id = sqlLastAutoId(conn); } dyStringFree(&dy); return id; } int doGenotype(struct lineFile *lf, struct sqlConnection *conn, char *taxon, int strainId, char *genotype) /* Unpack genotype string, alphabatize it, return id associated * with it if possible, otherwise create associated genes and * alleles then genotype, and return genotypeId. */ { int id = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); struct slName *geneAlleleList = commaSepToSlNames(genotype), *el; -struct dyString *alphabetical = newDyString(0); +struct dyString *alphabetical = dyStringNew(0); slSort(&geneAlleleList, slNameCmp); for (el = geneAlleleList; el != NULL; el = el->next) dyStringPrintf(alphabetical, "%s,", el->name); sqlDyStringPrintf(dy, "select id from genotype where "); sqlDyStringPrintf(dy, "taxon='%s' and strain=%d and alleles='%s'", taxon, strainId, alphabetical->string); id = sqlQuickNum(conn, dy->string); if (id == 0) { /* Create main genotype record. */ dyStringClear(dy); sqlDyStringPrintf(dy, "insert into genotype set"); sqlDyStringPrintf(dy, " id = default,\n"); @@ -882,31 +882,31 @@ static void veryClose(struct dyString *query, char *field, char *val) /* Append clause to query to select field to be within a very * small number to val (which is an ascii floating point value) */ { double x = atof(val); double e = 0.000001; sqlDyStringPrintf(query, "%s > %f and %s < %f and ", field, x-e, field, x+e); } int doSpecimen(struct lineFile *lf, struct sqlConnection *conn, char *name, char *taxon, int genotype, int bodyPart, int sex, char *age, char *minAge, char *maxAge, char *notes) /* Add specimen to table if it doesn't already exist. */ { int id = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); if (minAge[0] == 0) minAge = age; if (maxAge[0] == 0) maxAge = age; sqlDyStringPrintf(dy, "select id from specimen where "); sqlDyStringPrintf(dy, "name = '%s' and ", name); sqlDyStringPrintf(dy, "taxon = '%s' and ", taxon); sqlDyStringPrintf(dy, "genotype = %d and ", genotype); sqlDyStringPrintf(dy, "bodyPart = %d and ", bodyPart); sqlDyStringPrintf(dy, "sex = %d and ", sex); veryClose(dy, "age", age); veryClose(dy, "minAge", minAge); veryClose(dy, "maxAge", maxAge); sqlDyStringPrintf(dy, "notes = '%s'", notes); id = sqlQuickNum(conn, dy->string); if (id == 0) @@ -925,31 +925,31 @@ sqlDyStringPrintf(dy, "notes = '%s'", notes); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); id = sqlLastAutoId(conn); } dyStringFree(&dy); return id; } int doPreparation(struct lineFile *lf, struct sqlConnection *conn, int fixation, int embedding, int permeablization, int sliceType, char *notes) /* Add preparation to table if it doesn't already exist. */ { int id = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); sqlDyStringPrintf(dy, "select id from preparation where "); sqlDyStringPrintf(dy, "fixation = %d and ", fixation); sqlDyStringPrintf(dy, "embedding = %d and ", embedding); sqlDyStringPrintf(dy, "permeablization = %d and ", permeablization); sqlDyStringPrintf(dy, "sliceType = %d and ", sliceType); sqlDyStringPrintf(dy, "notes = '%s'", notes); id = sqlQuickNum(conn, dy->string); if (id == 0) { dyStringClear(dy); sqlDyStringPrintf(dy, "insert into preparation set"); sqlDyStringPrintf(dy, " id = default,\n"); sqlDyStringPrintf(dy, " fixation = %d,\n", fixation); sqlDyStringPrintf(dy, " embedding = %d,\n", embedding); @@ -958,31 +958,31 @@ sqlDyStringPrintf(dy, " notes = '%s'", notes); verbose(2, "%s\n", dy->string); sqlUpdate(conn, dy->string); id = sqlLastAutoId(conn); } dyStringFree(&dy); return id; } int doImage(struct lineFile *lf, struct sqlConnection *conn, int submissionSet, int imageFile, int imagePos, char *paneLabel, int sectionSet, int sectionIx, int specimen, int preparation) /* Update image table. */ { int id = 0; -struct dyString *dy = newDyString(0); +struct dyString *dy = dyStringNew(0); sqlDyStringPrintf(dy, "select id from image where "); sqlDyStringPrintf(dy, "submissionSet = %d and ", submissionSet); sqlDyStringPrintf(dy, "imageFile = %d and ", imageFile); sqlDyStringPrintf(dy, "imagePos = %d and ", imagePos); sqlDyStringPrintf(dy, "paneLabel = '%s' and ", paneLabel); sqlDyStringPrintf(dy, "sectionSet = %d and ", sectionSet); sqlDyStringPrintf(dy, "sectionIx = %d and ", sectionIx); sqlDyStringPrintf(dy, "specimen = %d and ", specimen); sqlDyStringPrintf(dy, "preparation = %d", preparation); id = sqlQuickNum(conn, dy->string); if (id == 0) { dyStringClear(dy); sqlDyStringPrintf(dy, "insert into image set ");