diff --git a/basepath.go b/basepath.go index 2e72793a..2212308b 100644 --- a/basepath.go +++ b/basepath.go @@ -55,12 +55,15 @@ func (b *BasePathFs) RealPath(name string) (path string, err error) { return name, err } - bpath := filepath.Clean(b.path) - path = filepath.Clean(filepath.Join(bpath, name)) - if !strings.HasPrefix(path, bpath) { + path = filepath.Join(b.path, name) + rel, err := filepath.Rel(b.path, path) + if err != nil { + // Usually occurs when files are on different drives. + return name, os.ErrNotExist + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return name, os.ErrNotExist } - return path, nil } diff --git a/basepath_test.go b/basepath_test.go index 828bb6a8..2ecc4400 100644 --- a/basepath_test.go +++ b/basepath_test.go @@ -39,52 +39,172 @@ func TestBasePathRoot(t *testing.T) { } func TestRealPath(t *testing.T) { - fs := NewOsFs() - baseDir, err := TempDir(fs, "", "base") - if err != nil { - t.Fatal("error creating tempDir", err) - } - defer fs.RemoveAll(baseDir) - anotherDir, err := TempDir(fs, "", "another") - if err != nil { - t.Fatal("error creating tempDir", err) - } - defer fs.RemoveAll(anotherDir) + tests := []struct { + name string + base string + input string + want string + expectErr bool + os string + }{ + // 1. Happy Paths + { + name: "Simple subpath", + base: "/var/data", + input: "file.txt", + want: "/var/data/file.txt", + }, + { + name: "Deeply nested subpath", + base: "/var/data", + input: "subdir/images/logo.png", + want: "/var/data/subdir/images/logo.png", + }, - bp := NewBasePathFs(fs, baseDir).(*BasePathFs) + // 2. Cleaning Behavior + { + name: "Cleans double slashes", + base: "/var/data", + input: "subdir//file.txt", + want: "/var/data/subdir/file.txt", + }, + { + name: "Cleans current directory dots", + base: "/var/data", + input: "./subdir/./file.txt", + want: "/var/data/subdir/file.txt", + }, + { + name: "Cleans subpath starts with /", + base: "/var/data", + input: "/file.txt", + want: "/var/data/file.txt", + }, + { + name: "Resolves internal dot-dot (safe)", + base: "/var/data", + input: "subdir/../file.txt", + want: "/var/data/file.txt", + }, + { + name: "Resolves base path dot-dot", + base: "/var/data/../data", + input: "file.txt", + want: "/var/data/file.txt", + }, - subDir := filepath.Join(baseDir, "s1") + // 3. Base Path is "." + { + name: "Base is dot, simple file", + base: ".", + input: "file.txt", + want: "file.txt", + }, + { + name: "Base is dot, input has dot prefix", + base: ".", + input: "./file.txt", + want: "file.txt", + }, + { + name: "Base is dot, safe traversal", + base: ".", + input: "foo/../bar", + want: "bar", + }, - realPath, err := bp.RealPath("/s1") - if err != nil { - t.Errorf("Got error %s", err) - } + // 4. paths starting with .. + { + name: "Valid file starting with .. (..X)", + base: "/var/data", + input: "..foo", + want: "/var/data/..foo", + }, + { + name: "Valid file named ...", + base: "/var/data", + input: "...", + want: "/var/data/...", + }, + { + name: "Hidden file", + base: "/var/data", + input: ".config", + want: "/var/data/.config", + }, + { + name: "Base is dot, input is ..foo", + base: ".", + input: "..foo", + want: "..foo", + }, - if realPath != subDir { - t.Errorf("Expected \n%s got \n%s", subDir, realPath) + // 5. Failure Cases + { + name: "Traversal out (parent)", + base: "/var/data", + input: "../etc/passwd", + expectErr: true, + }, + { + name: "Traversal out (root)", + base: "/var/data", + input: "../../../../etc/passwd", + expectErr: true, + }, + { + name: "Base is dot, traversal out", + base: ".", + input: "../file.txt", + expectErr: true, + }, + { + name: "Partial suffix match (e.g. /var/dataset vs /var/data)", + base: "/var/data", + input: "../dataset/file.txt", + expectErr: true, + }, + { + name: "Windows: Absolute path", + base: `C:\base`, + input: `C:\Windows\System32`, + expectErr: true, + os: "windows", + }, } - if runtime.GOOS == "windows" { - _, err = bp.RealPath(anotherDir) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.os != "" && tt.os != runtime.GOOS { + t.Skipf("Skipping test for OS %q", tt.os) + } - if err != os.ErrNotExist { - t.Errorf("Expected os.ErrNotExist") - } + baseFs := &MemMapFs{} + bpInterface := NewBasePathFs(baseFs, tt.base) + bp := bpInterface.(*BasePathFs) - } else { - // on *nix we have no way of just looking at the path and tell that anotherDir - // is not inside the base file system. - // The user will receive an os.ErrNotExist later. - surrealPath, err := bp.RealPath(anotherDir) - if err != nil { - t.Errorf("Got error %s", err) - } + got, err := bp.RealPath(tt.input) - expected := filepath.Join(baseDir, anotherDir) + if tt.expectErr { + if err == nil { + t.Errorf("expected error for input %q, got nil. Result was: %q", tt.input, got) + } + return + } - if surrealPath != expected { - t.Errorf("Expected \n%s got \n%s", expected, surrealPath) - } + if err != nil { + t.Fatalf("unexpected error for input %q: %v", tt.input, err) + } + + if runtime.GOOS == "windows" { + tt.want = filepath.FromSlash(tt.want) + } + + if got != tt.want { + t.Errorf("RealPath() mismatch.\nBase: %q\nInput: %q\nGot: %q\nWant: %q", + tt.base, tt.input, got, tt.want) + } + }) } }