f-denkena/rclone
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

lib/sanitize: factor untrusted path sanitization out of archive extract

Browse Source 
Move the archive entry name validation added for CVE-2026-59732 from
cmd/archive/extract into a new lib/sanitize package as sanitize.Path,
so the same check can be shared with the archive backend which mounts
archives as a filesystem.

sanitize.Path keeps the extract semantics - reject any name with a
".." path component, treating both "/" and "\" as separators - and
additionally cleans the name with path.Clean. This corrects two edge
cases in extract: a repeated "./" prefix ("././file.txt") is now fully
stripped rather than only the first, and a bare "." entry is now
treated as the archive root and skipped.

Add sanitize.Leaf, which rejects a name that is empty, ".", ".." or
contains a "/", for checking a single directory entry name read from
an archive.

The names handled are rclone remote paths, in which "/" is the only
separator and "\" an ordinary character, so Leaf does not reject a
backslash: making a name safe for its storage is the destination
backend's job (the local backend encodes "\" on Windows and refuses
paths which escape its root). Path's rejection of ".." between
backslashes is kept as defence in depth for extract.
This commit is contained in:
Nick Craig-Wood
2026-09-04 19:00:22 +01:00
parent 6453374403
commit 2b7d0b16ed
4 changed files with 241 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/archive/extract/extract.go
+11 -17
View File
@@ -8,7 +8,6 @@ import (
"errors"
"fmt"
"path"
"slices"
"strings"
"github.com/mholt/archives"
@@ -18,6 +17,7 @@ import (
"github.com/rclone/rclone/fs/accounting"
"github.com/rclone/rclone/fs/filter"
"github.com/rclone/rclone/fs/operations"
"github.com/rclone/rclone/lib/sanitize"
"github.com/spf13/cobra"
)
@@ -202,26 +202,20 @@ func ArchiveExtract(ctx context.Context, dst fs.Fs, dstDir string, src fs.Fs, sr
// destPath maps an archive entry name onto its destination remote within
// dstDir, returning an error if the name is unsafe.
//
// Archive entry names are attacker controlled. A leading "./" is stripped:
// tar archives created with relative paths (e.g. "tar -czf archive.tar.gz .")
// use "./" prefixed entries and, without stripping, rclone would encode the
// "." as a full-width dot character creating a spurious directory.
//
// Any entry with a ".." path component is then rejected to prevent a path
// traversal ("Zip Slip") attack: path.Join collapses "..", so an entry such
// as "../escaped.txt" joined onto "dir" would resolve to "escaped.txt" and be
// written outside the selected destination directory. Both "/" and "\" are
// treated as separators when looking for ".." segments: archive names should
// use "/", but a crafted archive may use "\", which the local backend treats
// as a path separator on Windows.
// Archive entry names are attacker controlled so they are sanitized with
// sanitize.Path before being joined onto dstDir. This cleans the name
// (stripping the "./" prefix which tar archives created with relative
// paths, e.g. "tar -czf archive.tar.gz .", put on their entries) and
// rejects any name with a ".." path component which would otherwise
// escape the destination directory (a path traversal, or "Zip Slip",
// attack).
//
// The returned remote is empty for the archive root entry ("./"), which the
// caller should skip.
func destPath(nameInArchive, dstDir string) (string, error) {
remote := strings.TrimPrefix(nameInArchive, "./")
isSeparator := func(r rune) bool { return r == '/' || r == '\\' }
if slices.Contains(strings.FieldsFunc(remote, isSeparator), "..") {
return "", fmt.Errorf("refusing to extract archive entry %q with a %q path component", nameInArchive, "..")
remote, err := sanitize.Path(nameInArchive)
if err != nil {
return "", fmt.Errorf("refusing to extract archive entry: %w", err)
}
if remote == "" {
return "", nil
cmd/archive/extract/extract_test.go
+2 -39
View File
@@ -18,25 +18,10 @@ func TestDestPath(t *testing.T) {
wantErr bool
}{
{
name: "strip leading dot-slash from file",
input: "./file.txt",
expected: "file.txt",
},
{
name: "strip leading dot-slash from nested path",
name: "sanitized name returned",
input: "./subdir/file.txt",
expected: "subdir/file.txt",
},
{
name: "no prefix unchanged",
input: "file.txt",
expected: "file.txt",
},
{
name: "nested path unchanged",
input: "dir/file.txt",
expected: "dir/file.txt",
},
{
name: "joined onto destination directory",
input: "file.txt",
@@ -48,44 +33,22 @@ func TestDestPath(t *testing.T) {
input: "./",
expected: "",
},
{
name: "only single leading dot-slash stripped",
input: "././file.txt",
expected: "./file.txt",
},
{
name: "leading dot-dot rejected",
input: "../etc/passwd",
wantErr: true,
},
{
name: "leading dot-dot rejected with destination",
input: "../escaped.txt",
dstDir: "safe/prefix",
wantErr: true,
},
{
name: "interior dot-dot rejected",
name: "interior dot-dot rejected with destination",
input: "dir/../../escaped.txt",
dstDir: "safe/prefix",
wantErr: true,
},
{
name: "trailing dot-dot rejected",
input: "dir/..",
wantErr: true,
},
{
name: "backslash dot-dot rejected",
input: `..\escaped.txt`,
wantErr: true,
},
{
name: "nested backslash dot-dot rejected",
input: `dir\..\..\escaped.txt`,
dstDir: "safe/prefix",
wantErr: true,
},
}
for _, tc := range tests {
lib/sanitize/sanitize.go
+59
View File
@@ -0,0 +1,59 @@
// Package sanitize cleans untrusted input before use.
//
// The names handled here are rclone remote paths, which use "/" as
// the only path separator. A "\" is an ordinary character in a remote
// path and is kept as such - it is up to each backend to make names
// safe for its own storage (the local backend, for example, encodes
// it on Windows and rejects any path which escapes its root). As
// defence in depth Path does however refuse names in which "\" would
// form a ".." component if it were a separator.
package sanitize
import (
"fmt"
"path"
"slices"
"strings"
)
// Leaf checks that the untrusted name is safe to use as a single path
// component (a directory entry leaf), such as a name read from an
// archive's directory listing.
//
// It returns an error if the name is empty, ".", "..", or contains a
// "/". Such a name would otherwise fabricate hierarchy or escape its
// directory when joined onto a path.
func Leaf(name string) error {
switch name {
case "", ".", "..":
return fmt.Errorf("unsafe path component %q", name)
}
if strings.Contains(name, "/") {
return fmt.Errorf("path component %q contains a \"/\"", name)
}
return nil
}
// Path sanitizes the untrusted "/"-separated path name, such as an
// archive entry name, so that it is safe to use as an rclone remote
// path relative to some root.
//
// It returns the name cleaned with path.Clean and with any leading
// and trailing "/" removed, or "" if the name refers to the root
// directory (e.g. "", "/" or "./").
//
// It returns an error for any name with a ".." path component, which
// would otherwise escape the root when joined onto it (a path
// traversal, or "Zip Slip", attack). Both "/" and "\" are treated as
// separators when looking for ".." components.
func Path(name string) (string, error) {
isSeparator := func(r rune) bool { return r == '/' || r == '\\' }
if slices.Contains(strings.FieldsFunc(name, isSeparator), "..") {
return "", fmt.Errorf("path %q has a %q component", name, "..")
}
cleaned := strings.Trim(path.Clean(name), "/")
if cleaned == "." {
return "", nil
}
return cleaned, nil
}
lib/sanitize/sanitize_test.go
+169
View File
@@ -0,0 +1,169 @@
package sanitize
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPath(t *testing.T) {
tests := []struct {
name string
input string
expected string
wantErr bool
}{
{
name: "plain file unchanged",
input: "file.txt",
expected: "file.txt",
},
{
name: "nested path unchanged",
input: "dir/file.txt",
expected: "dir/file.txt",
},
{
name: "strip leading dot-slash from file",
input: "./file.txt",
expected: "file.txt",
},
{
name: "strip leading dot-slash from nested path",
input: "./subdir/file.txt",
expected: "subdir/file.txt",
},
{
name: "strip repeated leading dot-slash",
input: "././file.txt",
expected: "file.txt",
},
{
name: "strip interior dot component",
input: "dir/./file.txt",
expected: "dir/file.txt",
},
{
name: "strip leading slash",
input: "/dir/file.txt",
expected: "dir/file.txt",
},
{
name: "strip trailing slash from directory",
input: "dir/",
expected: "dir",
},
{
name: "collapse doubled slashes",
input: "dir//file.txt",
expected: "dir/file.txt",
},
{
name: "empty name is the root",
input: "",
expected: "",
},
{
name: "dot-slash is the root",
input: "./",
expected: "",
},
{
name: "dot is the root",
input: ".",
expected: "",
},
{
name: "slash is the root",
input: "/",
expected: "",
},
{
name: "three dots allowed",
input: "dir/...",
expected: "dir/...",
},
{
name: "backslash kept in file name",
input: `dir/back\slash.txt`,
expected: `dir/back\slash.txt`,
},
{
name: "leading dot-dot rejected",
input: "../etc/passwd",
wantErr: true,
},
{
name: "interior dot-dot rejected",
input: "dir/../../escaped.txt",
wantErr: true,
},
{
name: "trailing dot-dot rejected",
input: "dir/..",
wantErr: true,
},
{
name: "bare dot-dot rejected",
input: "..",
wantErr: true,
},
{
name: "backslash dot-dot rejected",
input: `..\escaped.txt`,
wantErr: true,
},
{
name: "nested backslash dot-dot rejected",
input: `dir\..\..\escaped.txt`,
wantErr: true,
},
{
name: "mixed separator dot-dot rejected",
input: `dir/..\escaped.txt`,
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := Path(tc.input)
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tc.expected, got)
})
}
}
func TestLeaf(t *testing.T) {
tests := []struct {
name string
input string
wantErr bool
}{
{name: "plain name", input: "file.txt"},
{name: "name with dots", input: "..."},
{name: "hidden name", input: ".hidden"},
{name: "empty rejected", input: "", wantErr: true},
{name: "dot rejected", input: ".", wantErr: true},
{name: "dot-dot rejected", input: "..", wantErr: true},
{name: "slash rejected", input: "a/b", wantErr: true},
{name: "backslash allowed", input: `a\b`},
{name: "leading slash rejected", input: "/etc", wantErr: true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := Leaf(tc.input)
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
})
}
}