d968686107101a1824d34a007a19c8d18b368b78 angie Wed Jun 8 17:06:40 2016 -0700 Limit the length of gbMembers.keyList to avoid buffer overflow (and wasted space). refs #17327 notes 98, 100, 101. diff --git src/hg/lib/wikiLink.c src/hg/lib/wikiLink.c index fc7b2b6..572e87f 100644 --- src/hg/lib/wikiLink.c +++ src/hg/lib/wikiLink.c @@ -156,59 +156,69 @@ if (sameString(row[0], userName)) { struct slName *validKeyList = slNameListFromString(row[1], ','); isValid = slNameInListUseCase(validKeyList, key); } } sqlFreeResult(&sr); return isValid; } static void deleteKey(struct sqlConnection *conn, uint idx, char *key) /* Remove key from idx row's comma-separated keyList. */ { char query[2048]; sqlSafef(query, sizeof(query), "select keyList from gbMembers where idx = %u", idx); -char buf[1024]; +char buf[2048]; char *keyListStr = sqlQuickQuery(conn, query, buf, sizeof(buf)); if (isNotEmpty(keyListStr)) { struct slName *keyList = slNameListFromString(keyListStr, ','); struct slName *keyToDelete = slNameFind(keyList, key); if (keyToDelete) { slRemoveEl(&keyList, keyToDelete); char *newListStr = slNameListToString(keyList, ','); sqlSafef(query, sizeof(query), "update gbMembers set keyList='%s' where idx = %u", newListStr, idx); sqlUpdate(conn, query); } } } static void insertKey(struct sqlConnection *conn, uint idx, char *key) /* Add a new entry to gbMembers.keyList for idx. */ { char query[2048]; sqlSafef(query, sizeof(query), "select keyList from gbMembers where idx = %u", idx); -char buf[1024]; +char buf[2048]; char *keyListStr = sqlQuickQuery(conn, query, buf, sizeof(buf)); -if (isNotEmpty(keyListStr)) +if (isEmpty(keyListStr)) + sqlSafef(query, sizeof(query), "update gbMembers set keyList='%s' where idx = %u", key, idx); +else + { + // Orphaned keys can pile up when we test this feature by editing or deleting cookies, + // or when cookies aren't working properly. + // If there are many more keys in keyListStr than we would expect a user to have + // (e.g. 5 devices * 3 browsers = 15 keys, 28 chars per key = 420 chars, so say 1000 chars) + // then delete the oldest key(s) to avoid buffer overflow. + char *p; + while (strlen(keyListStr) > 1000 && (p = strrchr(keyListStr, ',')) != NULL) + *p = '\0'; sqlSafef(query, sizeof(query), "update gbMembers set keyList='%s,%s' where idx = %u", key, keyListStr, idx); -else - sqlSafef(query, sizeof(query), "update gbMembers set keyList='%s' where idx = %u", key, idx); + } sqlUpdate(conn, query); } char *getCookieDomainString() /* Get a string that will look something like " domain=.ucsc.edu;" if central.domain * is defined, otherwise just "". Don't free result. */ { static char domainString[256]; char *domain = cloneString(cfgOption(CFG_CENTRAL_DOMAIN)); if (domain != NULL && strchr(domain, '.') != NULL) safef(domainString, sizeof(domainString), " domain=%s;", domain); else domainString[0] = '\0'; return domainString; }