WebDAV: Harden joinUnderBase against drive-letter and backslash paths

Normalize separators, reject drive-letter prefixes, and verify
containment with filepath.Rel instead of a string prefix, matching the
pkg/fs ZIP safe-join. Extend TestJoinUnderBase to mirror TestSafeJoin
coverage and lock the handler surface via WebDAVFileName.
This commit is contained in:
Michael Mayer 2026-06-10 17:44:13 +00:00
parent b59ebb994b
commit 22c1789c14
2 changed files with 85 additions and 15 deletions

View file

@ -210,23 +210,40 @@ func WebDAVFileName(request *http.Request, router *gin.RouterGroup, conf *config
}
// joinUnderBase joins a base directory with a relative name and ensures
// that the resulting path stays within the base directory. Absolute
// paths and Windows-style volume names are rejected.
// that the resulting path stays within the base directory. Absolute paths,
// Windows-style volume names, and drive-letter prefixes are rejected, and
// containment is verified with filepath.Rel rather than a string prefix.
// This mirrors the hardened safe-join used for archive extraction in pkg/fs.
func joinUnderBase(baseDir, rel string) (string, error) {
if rel == "" {
return "", fmt.Errorf("invalid path")
}
// Normalize separators so mixed '/' and '\\' are handled consistently.
rel = strings.ReplaceAll(rel, "\\", "/")
// Reject Windows-style drive-letter prefixes even on non-Windows platforms.
if len(rel) >= 2 && rel[1] == ':' && ((rel[0] >= 'A' && rel[0] <= 'Z') || (rel[0] >= 'a' && rel[0] <= 'z')) {
return "", fmt.Errorf("invalid path: absolute or volume path not allowed")
}
// Reject absolute or volume paths.
if filepath.IsAbs(rel) || filepath.VolumeName(rel) != "" {
return "", fmt.Errorf("invalid path: absolute or volume path not allowed")
}
cleaned := filepath.Clean(rel)
// Compose destination and verify it stays inside base.
dest := filepath.Join(baseDir, cleaned)
base := filepath.Clean(baseDir)
if dest != base && !strings.HasPrefix(dest, base+string(os.PathSeparator)) {
// Compose destination and verify it stays inside base using filepath.Rel.
dest := filepath.Join(base, cleaned)
relToBase, err := filepath.Rel(base, dest)
if err != nil {
return "", fmt.Errorf("invalid path: %w", err)
} else if relToBase == ".." || strings.HasPrefix(relToBase, ".."+string(os.PathSeparator)) {
return "", fmt.Errorf("invalid path: outside base directory")
}
return dest, nil
}

View file

@ -18,16 +18,58 @@ import (
func TestJoinUnderBase(t *testing.T) {
base := t.TempDir()
// Normal join
out, err := joinUnderBase(base, "a/b/c.txt")
assert.NoError(t, err)
assert.Equal(t, filepath.Join(base, "a/b/c.txt"), out)
// Absolute rejected
_, err = joinUnderBase(base, "/etc/passwd")
assert.Error(t, err)
// Parent traversal rejected
_, err = joinUnderBase(base, "../../etc/passwd")
assert.Error(t, err)
t.Run("NormalJoin", func(t *testing.T) {
out, err := joinUnderBase(base, "a/b/c.txt")
assert.NoError(t, err)
assert.Equal(t, filepath.Join(base, "a/b/c.txt"), out)
})
t.Run("BackslashNormalized", func(t *testing.T) {
out, err := joinUnderBase(base, "a\\b\\c.txt")
assert.NoError(t, err)
assert.Equal(t, filepath.Join(base, "a/b/c.txt"), out)
})
t.Run("CleansInsideBase", func(t *testing.T) {
out, err := joinUnderBase(base, "dir/../evil.txt")
assert.NoError(t, err)
assert.Equal(t, filepath.Join(base, "evil.txt"), out)
})
t.Run("EmptyRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "")
assert.Error(t, err)
})
t.Run("AbsoluteRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "/etc/passwd")
assert.Error(t, err)
})
t.Run("RootedBackslashRejected", func(t *testing.T) {
// "\rooted\path.txt" normalizes to "/rooted/path.txt", caught as absolute.
_, err := joinUnderBase(base, "\\rooted\\path.txt")
assert.Error(t, err)
})
t.Run("ParentTraversalRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "../../etc/passwd")
assert.Error(t, err)
})
t.Run("BackslashTraversalRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "..\\..\\etc\\passwd")
assert.Error(t, err)
})
t.Run("DriveLetterForwardSlashRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "C:/drive.txt")
assert.Error(t, err)
})
t.Run("DriveLetterBackslashRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "C:\\drive.txt")
assert.Error(t, err)
})
t.Run("LowercaseDriveLetterRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "c:/drive.txt")
assert.Error(t, err)
})
t.Run("VolumeNameOnlyRejected", func(t *testing.T) {
_, err := joinUnderBase(base, "C:")
assert.Error(t, err)
})
}
func TestWebDAVFileName_PathTraversalRejected(t *testing.T) {
@ -55,6 +97,17 @@ func TestWebDAVFileName_PathTraversalRejected(t *testing.T) {
assert.Equal(t, insideFile, got)
}
func TestWebDAVFileName_DriveLetterRejected(t *testing.T) {
conf := newWebDAVTestConfig(t)
r := gin.New()
grp := r.Group(conf.BaseUri(WebDAVOriginals))
// A Windows drive-letter prefix must not resolve to an in-tree name.
req := &http.Request{Method: header.MethodPut}
req.URL = &url.URL{Path: conf.BaseUri(WebDAVOriginals) + "/C:/drive.txt"}
got := WebDAVFileName(req, grp, conf)
assert.Equal(t, "", got, "should reject drive-letter path")
}
func TestWebDAVFileName_MethodNotPut(t *testing.T) {
conf := newWebDAVTestConfig(t)
r := gin.New()