Skip to content

Commit c5031df

Browse files
committed
security: fix Windows path traversal in sanitizedPathJoin
Caught by FuzzSanitizedPathJoin on Windows CI: sanitizedPathJoin("", "/../../../etc/passwd") returned "..\..\etc\passwd" - the traversal escaped root instead of being neutralized. reqPath is an HTTP request path (always "/"-separated, regardless of host OS), but the code cleaned it with filepath.Clean, which uses native-separator, native-OS rules. On Windows, filepath.Clean does not treat a driveless "/"-rooted path as absolute, so a leading ".." isn't collapsed at the root the way it is on POSIX - it survives into the joined path instead of being dropped. Fixed by cleaning reqPath with the "path" package (POSIX-only, no OS-dependent branching) before handing it to filepath.Join for the native-separator join onto root. path.Clean deterministically produces the same traversal-free result on every platform, so there's no leftover ".." left for filepath's OS-specific rules to mishandle.
1 parent f0734f5 commit c5031df

1 file changed

Lines changed: 12 additions & 3 deletions

File tree

cgi.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"crypto/tls"
1919
"net"
2020
"net/http"
21+
"path"
2122
"path/filepath"
2223
"strings"
2324
"unicode/utf8"
@@ -338,17 +339,25 @@ func sanitizedPathJoin(root, reqPath string) string {
338339
root = "."
339340
}
340341

341-
path := filepath.Join(root, filepath.Clean("/"+reqPath))
342+
// reqPath is an HTTP request path: always "/"-separated, regardless of
343+
// host OS. It must be cleaned with the "path" package (POSIX-only),
344+
// not "path/filepath": on Windows, filepath.Clean does not treat a
345+
// driveless "/"-rooted path as absolute, so a leading ".." isn't
346+
// collapsed at the root the way it is on POSIX - it survives into the
347+
// joined path instead, escaping root.
348+
cleanedReqPath := filepath.FromSlash(path.Clean("/" + reqPath))
349+
350+
joined := filepath.Join(root, cleanedReqPath)
342351

343352
// filepath.Join also cleans the path, and cleaning strips
344353
// the trailing slash, so we need to re-add it afterward.
345354
// if the length is 1, then it's a path to the root,
346355
// and that should return ".", so we don't append the separator.
347356
if strings.HasSuffix(reqPath, "/") && len(reqPath) > 1 {
348-
path += separator
357+
joined += separator
349358
}
350359

351-
return path
360+
return joined
352361
}
353362

354363
// splitRemoteAddr splits "host:port" leniently: a missing port is accepted.

0 commit comments

Comments
 (0)