924c63fc1be1909f8ab2de587a968dfe1955f499 braney Tue Sep 8 07:19:24 2026 -0700 lib: replace eatExcessDotDotInPath with eatExcessDotsInPath, and resolve a root ".." simplifyPathToDir now canonicalizes with eatExcessDotsInPath instead of the older eatExcessDotDotInPath, which is removed. simplifyPathToDir was its only caller. The old routine scanned for the literal string "/../" and so gave several answers that do not match realpath(3): ../../a became a (the second .. ate the first) x/./../y became x/y (.. ate the ".", not the "x") ../.. became "" (an empty path, not the parent) a/./b stayed a/./b (single dots were never removed) h/../../../etc/passwd became etc/passwd (the escaping .. were eaten) The last one is the reason to bother. A caller that wants to know whether a path stays inside a directory cannot tell from the old result, because a path that climbs out comes back looking like it stayed in. Two changes to eatExcessDotsInPath came out of this. It now drops a ".." at the root of an absolute path, so /../a gives /a and /.. gives /, as realpath(3) does. Because an absolute path can then never hold a "..", the guard is exactly "nothing consumed and not absolute". It also returns "." rather than "" for a non-empty relative path that reduces to nothing. Callers join the result with "%s/%s", where an empty string names the file system root instead of the current directory. Without this, "tdbQuery -root=." looked for /tagTypes.tab and died. The in-place write is safe because a non-empty input always leaves room for one byte. The DEBUG selftest is rewritten. Two of its assertions asserted the old wrong answers for /.. and /../a, a third followed from them, and three asserted the empty-string result now replaced by ".". Added cases for each item above. Verified: all 38 selftest cases pass against the built library, and 48089 exhaustively enumerated paths over the alphabets "/.a" and "/.ab" match an independent model with no ASan or UBSan report. tdbQuery and raSqlQuery give byte-identical output to the master build across every root form tried, except two that the master build got wrong. The other caller of eatExcessDotsInPath is resolveDotDots, which hgTrackUi uses to canonicalize a fileUrl before checking it against a hub's base directory. Neither change loosens that check: a path that used to canonicalize to /../secret now gives /secret, and neither is under a hub base directory. refs #37263 diff --git src/lib/osunix.c src/lib/osunix.c index acb4045456b..3403abd8b38 100644 --- src/lib/osunix.c +++ src/lib/osunix.c @@ -414,36 +414,42 @@ s = d = path; char c, lastC = 0; while ((c = *s++) != 0) { if (c == '/' && lastC == c) continue; *d++ = c; lastC = c; } *d = 0; } void eatExcessDotsInPath(char *path) /* Remove . and .. components from path in place using two pointers. * Single dots are removed, double dots consume the preceding component - * unless it is also ".." or doesn't exist (relative path). + * unless it is also ".." or doesn't exist (relative path). A ".." at the + * root of an absolute path is dropped, so /../a becomes /a, matching + * realpath(3). Any trailing slash is removed. A non-empty relative path + * that reduces to nothing becomes ".", not the empty string, since callers + * join the result with "%s/%s" and an empty string there would name the + * file system root. An empty input stays empty. * Assumes no // in input (call eatSlashSlashInPath first). */ { char *src = path; char *dst = path; boolean absolute = (*src == '/'); +boolean wasEmpty = (*src == 0); if (absolute) *dst++ = *src++; while (*src) { /* Find end of this component */ char *compEnd = strchr(src, '/'); int compLen; if (compEnd) compLen = compEnd - src; else compLen = strlen(src); if (compLen == 1 && src[0] == '.') @@ -470,177 +476,154 @@ else prevStart = prevSlash + 1; int prevLen = (dst - prevStart); if (prevStart < dst && *(dst-1) == '/') prevLen--; /* exclude trailing slash from comparison */ /* Only consume if previous component is not ".." */ if (!(prevLen == 2 && prevStart[0] == '.' && prevStart[1] == '.')) { dst = prevStart; /* Also remove the preceding separator if present */ if (dst > path + (absolute ? 1 : 0) && *(dst-1) == '/') dst--; consumed = TRUE; } } - if (!consumed) + if (!consumed && !absolute) { - /* Write ".." forward */ - if (dst > path + (absolute ? 1 : 0)) + /* Nothing left to consume in a relative path, so keep the "..". + * An absolute path instead drops it, since /.. is / */ + if (dst > path) *dst++ = '/'; *dst++ = '.'; *dst++ = '.'; } src += compLen; if (*src == '/') src++; } else { /* Normal component: copy with leading slash separator if needed */ if (dst > path + (absolute ? 1 : 0)) *dst++ = '/'; memmove(dst, src, compLen); dst += compLen; src += compLen; if (*src == '/') src++; } } -*dst = 0; -} - -static void eatExcessDotDotInPath(char *path) -/* If there's a /.. in path take it out. Turns - * 'this/long/../dir/file' to 'this/dir/file - * and - * 'this/../file' to 'file' - * - * and - * 'this/long/..' to 'this' - * and - * 'this/..' to '' - * and - * /this/..' to '/' */ -{ -/* Take out each /../ individually */ -for (;;) - { - /* Find first bit that needs to be taken out. */ - char *excess= strstr(path, "/../"); - char *excessEnd = excess+4; - if (excess == NULL || excess == path) - break; - - /* Look for a '/' before this */ - char *excessStart = matchingCharBeforeInLimits(path, excess, '/'); - if (excessStart == NULL) /* Preceding '/' not found */ - excessStart = path; - else - excessStart += 1; - strcpy(excessStart, excessEnd); - } +/* "." rather than "" for something like "a/..", so the result stays a + * relative path. There is always room: the input was at least one byte. */ +if (!absolute && dst == path && !wasEmpty) + *dst++ = '.'; -/* Take out final /.. if any */ -if (endsWith(path, "/..")) - { - if (!sameString(path, "/..")) /* We don't want to turn this to blank. */ - { - int len = strlen(path); - char *excessStart = matchingCharBeforeInLimits(path, path+len-3, '/'); - if (excessStart == NULL) /* Preceding '/' not found */ - excessStart = path; - else - excessStart += 1; - *excessStart = 0; - } - } +*dst = 0; } char *simplifyPathToDir(char *path) -/* Return path with ~ and .. taken out. Also any // or trailing /. +/* Return path with ~, . and .. taken out. Also any // or trailing /. * freeMem result when done. */ { /* Expand ~ if any with result in newPath */ char newPath[PATH_LEN]; int newLen = 0; char *s = path; if (*s == '~') { char *homeDir = getenv("HOME"); if (homeDir == NULL) errAbort("No HOME environment var defined after ~ in simplifyPathToDir"); ++s; if (*s == '/') /* ~/something */ { ++s; safef(newPath, sizeof(newPath), "%s/", homeDir); } else /* ~something */ { safef(newPath, sizeof(newPath), "%s/../", homeDir); } newLen = strlen(newPath); } int remainingLen = strlen(s); if (newLen + remainingLen >= sizeof(newPath)) errAbort("path too big in simplifyPathToDir"); strcpy(newPath+newLen, s); -/* Remove //, .. and trailing / */ +/* Remove //, . , .. and trailing / */ eatSlashSlashInPath(newPath); -eatExcessDotDotInPath(newPath); -int lastPos = strlen(newPath)-1; -if (lastPos > 0 && newPath[lastPos] == '/') - newPath[lastPos] = 0; +eatExcessDotsInPath(newPath); return cloneString(newPath); } #ifdef DEBUG void simplifyPathToDirSelfTest() { /* First test some cases which should remain the same. */ assert(sameString(simplifyPathToDir(""),"")); assert(sameString(simplifyPathToDir("a"),"a")); assert(sameString(simplifyPathToDir("a/b"),"a/b")); assert(sameString(simplifyPathToDir("/"),"/")); -assert(sameString(simplifyPathToDir("/.."),"/..")); -assert(sameString(simplifyPathToDir("/../a"),"/../a")); /* Now test removing trailing slash. */ assert(sameString(simplifyPathToDir("a/"),"a")); assert(sameString(simplifyPathToDir("a/b/"),"a/b")); +/* Test . removal. */ +assert(sameString(simplifyPathToDir("."),".")); +assert(sameString(simplifyPathToDir("./"),".")); +assert(sameString(simplifyPathToDir("./a"),"a")); +assert(sameString(simplifyPathToDir("a/."),"a")); +assert(sameString(simplifyPathToDir("a/./b"),"a/b")); +assert(sameString(simplifyPathToDir("/a/./b"),"/a/b")); + /* Test .. removal. */ -assert(sameString(simplifyPathToDir("a/.."),"")); -assert(sameString(simplifyPathToDir("a/../"),"")); +assert(sameString(simplifyPathToDir("a/.."),".")); +assert(sameString(simplifyPathToDir("a/../"),".")); assert(sameString(simplifyPathToDir("a/../b"),"b")); assert(sameString(simplifyPathToDir("/a/.."),"/")); assert(sameString(simplifyPathToDir("/a/../"),"/")); assert(sameString(simplifyPathToDir("/a/../b"),"/b")); assert(sameString(simplifyPathToDir("a/b/.."),"a")); assert(sameString(simplifyPathToDir("a/b/../"),"a")); assert(sameString(simplifyPathToDir("a/b/../c"),"a/c")); assert(sameString(simplifyPathToDir("a/../b/../c"),"c")); -assert(sameString(simplifyPathToDir("a/../b/../c/.."),"")); +assert(sameString(simplifyPathToDir("a/../b/../c/.."),".")); assert(sameString(simplifyPathToDir("/a/../b/../c/.."),"/")); +assert(sameString(simplifyPathToDir("x/./../y"),"y")); + +/* A .. that climbs out of a relative path has to survive, so that a caller + * can tell "still inside" from "escaped". */ +assert(sameString(simplifyPathToDir(".."),"..")); +assert(sameString(simplifyPathToDir("../.."),"../..")); +assert(sameString(simplifyPathToDir("../a"),"../a")); +assert(sameString(simplifyPathToDir("../../a"),"../../a")); +assert(sameString(simplifyPathToDir("a/../../b"),"../b")); +assert(sameString(simplifyPathToDir("h/../../../etc/passwd"),"../../etc/passwd")); + +/* A .. at the root of an absolute path is dropped, as in realpath(3). */ +assert(sameString(simplifyPathToDir("/.."),"/")); +assert(sameString(simplifyPathToDir("/../a"),"/a")); +assert(sameString(simplifyPathToDir("/a/../../b"),"/b")); /* Test // removal */ assert(sameString(simplifyPathToDir("//"),"/")); -assert(sameString(simplifyPathToDir("//../"),"/..")); +assert(sameString(simplifyPathToDir("//../"),"/")); assert(sameString(simplifyPathToDir("a//b///c"),"a/b/c")); assert(sameString(simplifyPathToDir("a/b///"),"a/b")); } #endif /* DEBUG */ char *getUser() /* Get user name */ { uid_t uid = geteuid(); struct passwd *pw = getpwuid(uid); if (pw == NULL) errnoAbort("getUser: can't get user name for uid %d", (int)uid); return pw->pw_name; }