71475dc7c4a4a603ed7334eb4d27fbda8abdbdec braney Tue Sep 8 17:54:23 2026 -0700 cart: leave the trash directory out of the symlink-resolving path check Code review of 67700f0c574 found that resolving trashDir() does more than widen a check. trashDir() is the relative "../trash", so its resolved spelling is a different string, and sessionDataPathFromTrash() substitutes exactly the relative spelling. A path accepted as trash in its resolved spelling therefore rewrites to itself, and the callers treat the two as two files: saveTrackFile() opens the old one for reading and truncates the new one for writing, then unlinks the old name and points it at itself, and sessionDataSaveTrashFile() unlinks the file and then aborts when link() fails. Since ctfile_<db>, customComposite-<db> and hubQuickLift-<db> reach saveTrackFile() from a cart value, a session save could have destroyed another session's file. Resolving a relative directory also resolves it against the working directory of the process, so the answer moved with the caller: on the command line trashDir() is $JKTRASH or ".", which made anything under $TMPDIR or under cgi-bin count as trash. Put isTrashPath() back on the plain string compare and keep the resolving version for sessionDataDir, sessionDataDirOld and myVariantsDataDir, which are the configured absolute directories the #38303 sessions are stored under. Only resolve an absolute directory, so the result no longer depends on the working directory. All 297,487 saved sessions in the September dumps of the RR, hgwbeta and hgwdev centrals were checked: none holds a resolved trash path, so nothing loses the widening. refs #38303, refs #38304, refs #37623 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> diff --git src/hg/lib/trashDir.c src/hg/lib/trashDir.c index 8b1d9967939..2d99ad1084b 100644 --- src/hg/lib/trashDir.c +++ src/hg/lib/trashDir.c @@ -36,47 +36,57 @@ return FALSE; int dirLen = strlen(dir); while (dirLen > 0 && dir[dirLen-1] == '/') dirLen -= 1; if (dirLen == 0) return FALSE; if (strncmp(path, dir, dirLen) != 0 || path[dirLen] != '/' || path[dirLen+1] == '\0') return FALSE; return !hasDotDotComponent(path + dirLen + 1); } static boolean pathIsUnderDirOrItsTarget(char *dir, char *path) /* pathIsUnderDir(), but also accept a path under the directory that dir resolves to. A * configured directory is often reached through a symlink, and sessionData.c stores the * resolved spelling of a path whose file is already a symlink, so both spellings turn up in - * saved sessions. Only dir is resolved. Resolving path would defeat the check, because a - * trash file is often a symlink into session storage on purpose. */ + * saved sessions. + * + * Only dir is resolved. Resolving path would defeat the check, because a trash file is + * often a symlink into session storage on purpose. Only an absolute dir is resolved, so + * the answer cannot depend on the working directory of the process. + * + * Only this one direction is covered: a dir configured as the already-resolved spelling + * does not accept a path written through the symlink. + * + * Do not use this on trashDir(). It is a relative path, "../trash", and + * sessionDataPathFromTrash() substitutes exactly that spelling, so accepting the resolved + * spelling here would hand that function a path it cannot rewrite. */ { if (pathIsUnderDir(dir, path)) return TRUE; -if (isEmpty(dir)) +if (isEmpty(dir) || dir[0] != '/') return FALSE; char resolved[PATH_MAX]; if (realpath(dir, resolved) == NULL) return FALSE; return pathIsUnderDir(resolved, path); } boolean isTrashPath(char *path) /* Return TRUE if path names a file inside the trash directory. */ { -return pathIsUnderDirOrItsTarget(trashDir(), path); +return pathIsUnderDir(trashDir(), path); } boolean isTrashOrSessionDataPath(char *path) /* Return TRUE if path is under the trash directory, or under one of the durable session-data * directories that trash files are moved to when a session is saved. */ { return isTrashPath(path) || pathIsUnderDirOrItsTarget(cfgOption("sessionDataDir"), path) || pathIsUnderDirOrItsTarget(cfgOption("sessionDataDirOld"), path); } boolean isServerUserFilePath(char *path) /* Return TRUE if path is under one of the directories where the server keeps files it made * for a user: the trash directory, the session-data directories, or a per-feature data * directory such as myVariantsDataDir. */