954265a3a58f3a3a18475faf6106b4b9d485c696 markd Sun Aug 16 21:47:53 2026 -0700 udc: resolve '.' and '..' in a remote URL instead of rejecting them. refs #38120 The #38056 cache-escape fix rejected any ".." path component when udc turns a remote URL into a cache path. That also broke hubs doing something legal: a relative trackDb or bigDataUrl that reaches up a level so two assemblies can share a file. The NHGRI T2T hub hit it four times in one afternoon on dev with TMP.MAT/../HG002v1.1/rep.trackDb.txt. Resolve the dot components the way the remote server resolves them and abort only when ".." would climb above the host. The escape #38056 closed stays closed and keeps the same error message, and the cache path now names the resource actually fetched, so a/x/../b and a/b share one cache entry. simplifyPathToDir in portable.h was not reusable here: it expands ~, eats //, strips a trailing /, and leaves /.. alone rather than catching the climb. Cache paths for URLs with no dot component are byte for byte unchanged, which matters because a change there would invalidate every cached file on every node. Verified by deriving cache paths for 16 URLs across the change; the only lines that differ are a/x/../b/file.txt and a/b/.. which used to abort, and a/./b/file.txt which used to cache under a path containing the dot. That last one orphans any existing cache entry for a URL with a "." component, costing one re-download; such URLs are rare and the new path is the correct one. diff --git src/lib/udc.c src/lib/udc.c index 4049b899bb4..d67fe5e39cb 100644 --- src/lib/udc.c +++ src/lib/udc.c @@ -1250,50 +1250,114 @@ { *ptr = 0; addElementToDy(dy, maxLen, name); dyStringAppend(dy, "/"); name = ptr + 1; ptr = strchr(name, '/'); } addElementToDy(dy, maxLen, name); return dyStringCannibalize(&dy); } +static struct slName *pathComponents(char *path) +/* Split path on '/', keeping empty components so the path can be rebuilt exactly. */ +{ +struct slName *list = NULL; +char *start = path, *slash; +while ((slash = strchr(start, '/')) != NULL) + { + slAddHead(&list, slNameNewN(start, slash - start)); + start = slash + 1; + } +slAddHead(&list, slNameNew(start)); +slReverse(&list); +return list; +} + +static struct slName *popPathComponent(struct slName *stack, char *protocol, char *afterProtocol) +/* Remove the component that a ".." applies to. The host is the one component that can not be + * removed; climbing past it is both an illegal remote path and the cache escape of #38056. */ +{ +if (stack == NULL || stack->next == NULL) + errAbort("Illegal '..' in the path of a remote URL: %s://%s", protocol, afterProtocol); +struct slName *popped = stack; +stack = stack->next; +freeMem(popped); +return stack; +} + +static char *joinPathComponents(struct slName *components) +/* Glue components back together with '/' between them. */ +{ +struct dyString *dy = dyStringNew(128); +struct slName *comp; +for (comp = components; comp != NULL; comp = comp->next) + { + if (comp != components) + dyStringAppendC(dy, '/'); + dyStringAppend(dy, comp->name); + } +return dyStringCannibalize(&dy); +} + +static char *resolveDotsInPath(char *protocol, char *afterProtocol) +/* Return afterProtocol with "." and ".." components resolved the way the remote server resolves + * them, so the cache path names the resource that is actually fetched. A path with no "." or + * ".." component comes back byte for byte the same, which matters because a change there would + * invalidate every cached file on every node. */ +{ +struct slName *components = pathComponents(afterProtocol); +struct slName *resolved = NULL; /* built in reverse, so its head is the last component */ +struct slName *comp; +for (comp = components; comp != NULL; comp = comp->next) + { + if (sameString(comp->name, "..")) + resolved = popPathComponent(resolved, protocol, afterProtocol); + else if (!sameString(comp->name, ".")) + slAddHead(&resolved, slNameNew(comp->name)); + } +slReverse(&resolved); +char *path = joinPathComponents(resolved); +slFreeList(&components); +slFreeList(&resolved); +return path; +} + void udcPathAndFileNames(struct udcFile *file, char *cacheDir, char *protocol, char *afterProtocol) /* Initialize udcFile path and names */ { if (cacheDir==NULL) return; -/* SECURITY (refs #38056): this is where a remote URL becomes a local path, so it is the - * place to make sure the URL cannot climb out of the cache. qEscaped deliberately leaves - * '.' and '/' alone so cached names stay readable, and longDirHash only rewrites - * over-long components, so a ".." in the URL survives all the way into cacheDir. From - * there makeDirsOnPath would create directories outside the cache root and we would write - * bitmap and sparseData files into them. A custom track or hub bigDataUrl reaches here, - * so this is anonymous. Reject the ".." path component; note that names merely starting - * with dots, like "..foo", are fine and must keep working. */ -if (startsWith("../", afterProtocol) || stringIn("/../", afterProtocol) != NULL || - endsWith(afterProtocol, "/..") || sameString(afterProtocol, "..")) - errAbort("Illegal '..' in the path of a remote URL: %s://%s", protocol, afterProtocol); -char *hashedAfterProtocol = longDirHash(cacheDir, afterProtocol); +/* SECURITY (refs #38056, #38120): this is where a remote URL becomes a local path, so it is + * the place to make sure the URL cannot climb out of the cache. qEscaped deliberately leaves + * '.' and '/' alone so cached names stay readable, and longDirHash only rewrites over-long + * components, so a ".." in the URL survives all the way into cacheDir. From there + * makeDirsOnPath would create directories outside the cache root and we would write bitmap + * and sparseData files into them. A custom track or hub bigDataUrl reaches here, so this is + * anonymous. Resolve the dot components the way the remote server does and abort only on the + * ones that climb above the host; a hub reaching up a level to share a file between assemblies + * is legal and has to keep working, as do names merely starting with dots like "..foo". */ +char *resolvedPath = resolveDotsInPath(protocol, afterProtocol); +char *hashedAfterProtocol = longDirHash(cacheDir, resolvedPath); int len = strlen(cacheDir) + 1 + strlen(protocol) + 1 + strlen(hashedAfterProtocol) + 1; file->cacheDir = needMem(len); safef(file->cacheDir, len, "%s/%s/%s", cacheDir, protocol, hashedAfterProtocol); +freeMem(resolvedPath); verbose(4, "UDC dir: %s\n", file->cacheDir); /* Create file names for bitmap and data portions. */ file->bitmapFileName = fileNameInCacheDir(file, bitmapName); file->sparseFileName = fileNameInCacheDir(file, sparseDataName); file->redirFileName = fileNameInCacheDir(file, redirName); file->resolvedFileName = fileNameInCacheDir(file, resolvedName); } static long long int udcSizeAndModTimeFromBitmap(char *bitmapFileName, time_t *retTime) /* Look up the file size from the local cache bitmap file, or -1 if there * is no cache for url. If retTime is non-null, store the remote update time in it. */ { long long int ret = -1; struct udcBitmap *bits = udcBitmapOpen(bitmapFileName);