From e509b3a5958f49144ece02eefc9649303d92ea0d Mon Sep 17 00:00:00 2001 From: Mani Kaustubh Mathur Date: Wed, 22 Jul 2026 21:13:02 +0000 Subject: [PATCH] fix: make staging path-traversal check separator-aware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit localPathIsAllowed guarded against staging PUT/GET escaping the configured stagingAllowedLocalPaths by rejecting a relative path containing the literal "../". filepath.Rel returns OS-native separators, so on Windows an escaping path is "..\\x", which the "../" substring never matches — the guard returned true (allowed) for a path outside the allowlist. This is a fail-open path-traversal check on Windows, present since the driver's first commit. Compare the relative path against filepath.Separator instead: a path is inside the allowed dir iff it is neither ".." nor starts with ".."+separator. Using a prefix check (not Contains) also stops a sibling like "..foo" from being mistaken for an escape. Only the default Thrift path reaches this code; the kernel/SEA backend rejects staging statements before execStagingOperation. Surfaced by the new macOS/Windows CI matrix — the existing TestPathAllowed now passes on Windows. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur --- connection.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/connection.go b/connection.go index 6f5a9cee..61bfb183 100644 --- a/connection.go +++ b/connection.go @@ -716,7 +716,15 @@ func localPathIsAllowed(stagingAllowedLocalPaths []string, localFile string) boo if err != nil { return false } - if !strings.Contains(relativePath, "../") { + // localFile is inside the allowed dir iff the relative path does not climb + // out of it, i.e. it is neither ".." itself nor starts with "..". + // filepath.Rel returns OS-native separators ("../x" on unix, "..\\x" on + // windows), so this must compare against filepath.Separator rather than a + // hard-coded "../" — the latter never matches the windows "..\\" form and + // silently allows an escaping path (a fail-open path-traversal check on + // windows). Using the "../" prefix (not a Contains) also avoids treating a + // sibling like "..foo" as an escape. + if relativePath != ".." && !strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) { return true } }