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/cgilib/wikiTrack.c src/hg/cgilib/wikiTrack.c index ce3bd32..fcae8f4 100644 --- src/hg/cgilib/wikiTrack.c +++ src/hg/cgilib/wikiTrack.c @@ -115,35 +115,35 @@ el = wikiTrackLoad(row); slAddHead(&list, el); } slReverse(&list); sqlFreeResult(&sr); return list; } void wikiTrackSaveToDb(struct sqlConnection *conn, struct wikiTrack *el, char *tableName, int updateSize) /* Save wikiTrack as a row to the table specified by tableName. * As blob fields may be arbitrary size updateSize specifies the approx size * of a string that would contain the entire query. Arrays of native types are * converted to comma separated strings and loaded as such, User defined types are * inserted as NULL. Strings are automatically escaped to allow insertion into the database. */ { -struct dyString *update = newDyString(updateSize); +struct dyString *update = dyStringNew(updateSize); sqlDyStringPrintf(update, "insert into %s values ( %u,'%s',%u,%u,'%s',%u,'%s','%s','%s','%s','%s','%s','%s','%s',%u,'%s')", tableName, el->bin, el->chrom, el->chromStart, el->chromEnd, el->name, el->score, el->strand, el->db, el->owner, el->color, el->class, el->creationDate, el->lastModifiedDate, el->descriptionKey, el->id, el->geneSymbol); sqlUpdate(conn, update->string); -freeDyString(&update); +dyStringFree(&update); } struct wikiTrack *wikiTrackCommaIn(char **pS, struct wikiTrack *ret) /* Create a wikiTrack out of a comma separated string. * This will fill in ret if non-null, otherwise will * return a new wikiTrack */ { char *s = *pS; if (ret == NULL) AllocVar(ret); ret->bin = sqlUnsignedComma(&s); ret->chrom = sqlStringComma(&s); ret->chromStart = sqlUnsignedComma(&s); @@ -521,50 +521,50 @@ { char *stripped = cloneString(rendered); char *found = NULL; char *begin = "<div class=\"editsection\""; char *end = "></a>"; /* XXXX is this response going to be language dependent ? */ if (stringIn(WIKI_NO_TEXT_RESPONSE,rendered)) return NULL; /* remove any edit sections */ while (NULL != (found = stringBetween(begin, end, stripped)) ) { if (strlen(found) > 0) { - struct dyString *rm = newDyString(1024); + struct dyString *rm = dyStringNew(1024); dyStringPrintf(rm, "%s%s%s", begin, found, end); stripString(stripped, rm->string); freeMem(found); - freeDyString(&rm); + dyStringFree(&rm); } } /* and remove comment strings from the wiki */ begin = "<!--"; end = "-->"; while (NULL != (found = stringBetween(begin, end, stripped)) ) { if (strlen(found) > 0) { - struct dyString *rm = newDyString(1024); + struct dyString *rm = dyStringNew(1024); dyStringPrintf(rm, "%s%s%s", begin, found, end); stripString(stripped, rm->string); freeMem(found); - freeDyString(&rm); + dyStringFree(&rm); } } return stripped; } void htmlCloneFormVarSet(struct htmlForm *parent, struct htmlForm *clone, char *name, char *val) /* clone form variable from parent, with new value, * if *val is NULL, clone val from parent */ { struct htmlFormVar *cloneVar; struct htmlFormVar *var; if (parent == NULL) @@ -583,79 +583,79 @@ cloneVar->curVal = cloneString(var->curVal); else cloneVar->curVal = cloneString(val); slAddHead(&clone->vars, cloneVar); } char *fetchWikiRawText(char *descriptionKey) /* fetch page from wiki in raw form as it is in the edit form */ { char wikiPageUrl[512]; safef(wikiPageUrl, sizeof(wikiPageUrl), "%s/index.php?title=%s&action=raw", cfgOptionDefault(CFG_WIKI_URL, NULL), descriptionKey); struct lineFile *lf = netLineFileMayOpen(wikiPageUrl); -struct dyString *wikiPage = newDyString(1024); +struct dyString *wikiPage = dyStringNew(1024); if (lf) { char *line; int lineSize; while (lineFileNext(lf, &line, &lineSize)) dyStringPrintf(wikiPage, "%s\n", line); lineFileClose(&lf); } /* test for text, remove any edit sections and comment strings */ char *rawText = NULL; if (wikiPage->string) { /* XXXX is this response going to be language dependent ? */ if (stringIn(WIKI_NO_TEXT_RESPONSE,wikiPage->string)) return NULL; rawText = dyStringCannibalize(&wikiPage); } -freeDyString(&wikiPage); +dyStringFree(&wikiPage); return rawText; } char *fetchWikiRenderedText(char *descriptionKey) /* fetch page from wiki in rendered form, strip it of edit URLs, * html comments, and test for actual proper return. * returned string can be freed after use */ { char wikiPageUrl[512]; safef(wikiPageUrl, sizeof(wikiPageUrl), "%s/index.php/%s?action=render", cfgOptionDefault(CFG_WIKI_URL, NULL), descriptionKey); struct lineFile *lf = netLineFileMayOpen(wikiPageUrl); -struct dyString *wikiPage = newDyString(1024); +struct dyString *wikiPage = dyStringNew(1024); if (lf) { char *line; int lineSize; while (lineFileNext(lf, &line, &lineSize)) dyStringPrintf(wikiPage, "%s\n", line); lineFileClose(&lf); } /* test for text, remove any edit sections and comment strings */ char *strippedRender = NULL; if (wikiPage->string) strippedRender = stripEditURLs(wikiPage->string); -freeDyString(&wikiPage); +dyStringFree(&wikiPage); return strippedRender; } char *adjustWikiUrls (char *comments) /* change relative to full urls for images */ { char *com; char buf[128]; char *url = cfgOptionDefault(CFG_WIKI_URL, NULL); safef(buf, sizeof(buf), "img src=\"%s/images", url); com = replaceChars(comments, "img src=\"/images", buf); return com; } @@ -708,31 +708,31 @@ /* the submit on the edit is going to need these cookies */ page->cookies = cookie; return (page); } void prefixComments(struct wikiTrack *item, char *comments, char *userName, char *seqName, int winStart, int winEnd, char *database, char *extraHeader, char *extraTag, char *category) /* add comments at the beginning of an existing wiki item */ { /* do nothing if given nothing */ if (NULL == comments) return; -struct dyString *content = newDyString(1024); +struct dyString *content = dyStringNew(1024); struct htmlPage *page = fetchEditPage(item->descriptionKey); /* create a stripped down edit form, we don't want all the variables */ struct htmlForm *strippedEditForm; AllocVar(strippedEditForm); struct htmlForm *currentEditForm = htmlFormGet(page,"editform"); if (NULL == currentEditForm) errAbort("prefixComments: can not get editform ? (wiki login confused ?)"); strippedEditForm->name = cloneString(currentEditForm->name); /* the lower case "post" in the editform does not work ? */ /*strippedEditForm->method = cloneString(currentEditForm->method);*/ strippedEditForm->method = cloneString("POST"); @@ -757,31 +757,31 @@ * removed, and then restore them. */ if (!(wpTextbox1->curVal && (strlen(wpTextbox1->curVal) > 2))) { char position[128]; char *newPos; char *userSignature; /* In the case where this is a restoration of the header lines, * may be a different creator than this user adding comments. * So, get the header line correct to represent the actual creator. */ if (sameWord(userName, item->owner)) userSignature = cloneString("~~~~"); else { - struct dyString *tt = newDyString(1024); + struct dyString *tt = dyStringNew(1024); dyStringPrintf(tt, "[[User:%s|%s]] ", item->owner, item->owner); dyStringPrintf(tt, "%s", item->creationDate); userSignature = dyStringCannibalize(&tt); } safef(position, 128, "%s:%d-%d", seqName, winStart+1, winEnd); newPos = addCommasToPos(database, position); if (extraHeader) { dyStringPrintf(content, "%s\n%s\n", category, extraHeader); } else { dyStringPrintf(content, "%s\n" @@ -828,41 +828,41 @@ char *wikiHost = wikiLinkHost(); /* fake out htmlPageFromForm since it doesn't understand the colon : */ safef(newUrl, ArraySize(newUrl), "http://%s%s", wikiHost, currentEditForm->action); freeMem(wikiHost); /* something, somewhere encoded the & into & which does not work */ char *fixedString = replaceChars(newUrl, "&", "&"); currentEditForm->action = cloneString(fixedString); strippedEditForm->action = cloneString(fixedString); struct htmlPage *editPage = htmlPageFromForm(page,strippedEditForm,"submit", "Submit"); freeMem(fixedString); if (NULL == editPage) errAbort("prefixComments: the edit is failing ?"); -freeDyString(&content); +dyStringFree(&content); } /* void prefixComments() */ void addDescription(struct wikiTrack *item, char *userName, char *seqName, int winStart, int winEnd, struct cart *cart, char *database, char *extraHeader, char *extraTag, char *category) /* add description to the end of an existing wiki item */ { char *newComments = cartNonemptyString(cart, NEW_ITEM_COMMENT); -struct dyString *content = newDyString(1024); +struct dyString *content = dyStringNew(1024); /* was nothing changed in the add comments entry box ? */ if (sameWord(ADD_ITEM_COMMENT_DEFAULT,newComments)) return; struct htmlPage *page = fetchEditPage(item->descriptionKey); /* create a stripped down edit form, we don't want all the variables */ struct htmlForm *strippedEditForm; AllocVar(strippedEditForm); struct htmlForm *currentEditForm = htmlFormGet(page,"editform"); if (NULL == currentEditForm) errAbort("addDescription: can not get editform ? (wiki login confused ?)"); @@ -894,31 +894,31 @@ } else { boolean recreateHeader = FALSE; char position[128]; char *newPos; char *userSignature; /* In the case where this is a restoration of the header lines, * may be a different creator than this user adding comments. * So, get the header line correct to represent the actual creator. */ if (sameWord(userName, item->owner)) userSignature = cloneString("~~~~"); else { - struct dyString *tt = newDyString(1024); + struct dyString *tt = dyStringNew(1024); dyStringPrintf(tt, "[[User:%s|%s]] ", item->owner, item->owner); dyStringPrintf(tt, "%s", item->creationDate); userSignature = dyStringCannibalize(&tt); recreateHeader = TRUE; } safef(position, 128, "%s:%d-%d", seqName, winStart+1, winEnd); newPos = addCommasToPos(database, position); if (extraHeader) { dyStringPrintf(content, "%s\n%s\n", category, extraHeader); } else { dyStringPrintf(content, "%s\n" @@ -968,31 +968,31 @@ char *wikiHost = wikiLinkHost(); /* fake out htmlPageFromForm since it doesn't understand the colon : */ safef(newUrl, ArraySize(newUrl), "http://%s%s", wikiHost, currentEditForm->action); freeMem(wikiHost); /* something, somewhere encoded the & into & which does not work */ char *fixedString = replaceChars(newUrl, "&", "&"); currentEditForm->action = cloneString(fixedString); strippedEditForm->action = cloneString(fixedString); struct htmlPage *editPage = htmlPageFromForm(page,strippedEditForm,"submit", "Submit"); freeMem(fixedString); if (NULL == editPage) errAbort("addDescription: the edit is failing ?"); -freeDyString(&content); +dyStringFree(&content); } /* void addDescription() */ char *encodedReturnUrl(char *(*hgUrl)()) /* Return a CGI-encoded URL with hgsid. Free when done. * The given function hgUrl() will construct the actual cgi binary URL */ { char retBuf[1024]; safef(retBuf, sizeof(retBuf), "http://%s/%s", cgiServerName(), (*hgUrl)()); return cgiEncode(retBuf); } boolean emailVerified(boolean showMessage) /* TRUE indicates email has been verified for this wiki user */ {