Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for Windows #9

Merged
merged 1 commit into from
Sep 6, 2023
+54 −37
Merged

Fix support for Windows #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion join.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,27 @@ func IsNotExist(err error) bool {
// components in the returned string are not modified (in other words are not
// replaced with symlinks on the filesystem) after this function has returned.
// Such a symlink race is necessarily out-of-scope of SecureJoin.
//
// Volume names in unsafePath are always discarded, regardless if they are
// provided via direct input or when evaluating symlinks. Therefore:
//
// "C:\Temp" + "D:\path\to\file.txt" results in "C:\Temp\path\to\file.txt"
func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
// Use the os.* VFS implementation if none was specified.
if vfs == nil {
vfs = osVFS{}
}

unsafePath = filepath.FromSlash(unsafePath)
var path bytes.Buffer
n := 0
for unsafePath != "" {
if n > 255 {
return "", &os.PathError{Op: "SecureJoin", Path: root + "/" + unsafePath, Err: syscall.ELOOP}
return "", &os.PathError{Op: "SecureJoin", Path: root + string(filepath.Separator) + unsafePath, Err: syscall.ELOOP}
}

if v := filepath.VolumeName(unsafePath); v != "" {
unsafePath = unsafePath[len(v):]
}

// Next path component, p.
Expand Down
79 changes: 43 additions & 36 deletions join_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"syscall"
"testing"
)
Expand All @@ -22,6 +23,11 @@ func symlink(t *testing.T, oldname, newname string) {
}
}

type input struct {
root, unsafe string
expected string
}

// Test basic handling of symlink expansion.
func TestSymlink(t *testing.T) {
dir, err := ioutil.TempDir("", "TestSymlink")
Expand All @@ -38,21 +44,26 @@ func TestSymlink(t *testing.T) {
symlink(t, "../../../../../../../../../../../../../etc", filepath.Join(dir, "etclink"))
symlink(t, "/../../../../../../../../../../../../../etc/passwd", filepath.Join(dir, "passwd"))

for _, test := range []struct {
root, unsafe string
expected string
}{
rootOrVol := string(filepath.Separator)
if vol := filepath.VolumeName(dir); vol != "" {
rootOrVol = vol + rootOrVol
}

tc := []input{
// Make sure that expansion with a root of '/' proceeds in the expected fashion.
{"/", filepath.Join(dir, "passwd"), "/etc/passwd"},
{"/", filepath.Join(dir, "etclink"), "/etc"},
{"/", filepath.Join(dir, "etc"), filepath.Join(dir, "somepath")},
{rootOrVol, filepath.Join(dir, "passwd"), filepath.Join(rootOrVol, "etc", "passwd")},
{rootOrVol, filepath.Join(dir, "etclink"), filepath.Join(rootOrVol, "etc")},

{rootOrVol, filepath.Join(dir, "etc"), filepath.Join(dir, "somepath")},
// Now test scoped expansion.
{dir, "passwd", filepath.Join(dir, "somepath", "passwd")},
{dir, "etclink", filepath.Join(dir, "somepath")},
{dir, "etc", filepath.Join(dir, "somepath")},
{dir, "etc/test", filepath.Join(dir, "somepath", "test")},
{dir, "etc/test/..", filepath.Join(dir, "somepath")},
} {
}

for _, test := range tc {
got, err := SecureJoin(test.root, test.unsafe)
if err != nil {
t.Errorf("securejoin(%q, %q): unexpected error: %v", test.root, test.unsafe, err)
Expand Down Expand Up @@ -85,29 +96,31 @@ func TestNoSymlink(t *testing.T) {
}
defer os.RemoveAll(dir)

for _, test := range []struct {
root, unsafe string
}{
// TODO: Do we need to have some conditional FromSlash handling here?
{dir, "somepath"},
{dir, "even/more/path"},
{dir, "/this/is/a/path"},
{dir, "also/a/../path/././/with/some/./.././junk"},
{dir, "yetanother/../path/././/with/some/./.././junk../../../../../../../../../../../../etc/passwd"},
{dir, "/../../../../../../../../../../../../../../../../etc/passwd"},
{dir, "../../../../../../../../../../../../../../../../somedir"},
{dir, "../../../../../../../../../../../../../../../../"},
{dir, "./../../.././././../../../../../../../../../../../../../../../../etc passwd"},
} {
expected := filepath.Join(test.root, filepath.Clean(string(filepath.Separator)+test.unsafe))
tc := []input{
{dir, "somepath", filepath.Join(dir, "somepath")},
{dir, "even/more/path", filepath.Join(dir, "even", "more", "path")},
{dir, "/this/is/a/path", filepath.Join(dir, "this", "is", "a", "path")},
{dir, "also/a/../path/././/with/some/./.././junk", filepath.Join(dir, "also", "path", "with", "junk")},
{dir, "yetanother/../path/././/with/some/./.././junk../../../../../../../../../../../../etc/passwd", filepath.Join(dir, "etc", "passwd")},
{dir, "/../../../../../../../../../../../../../../../../etc/passwd", filepath.Join(dir, "etc", "passwd")},
{dir, "../../../../../../../../../../../../../../../../somedir", filepath.Join(dir, "somedir")},
{dir, "../../../../../../../../../../../../../../../../", filepath.Join(dir)},
{dir, "./../../.././././../../../../../../../../../../../../../../../../etc passwd", filepath.Join(dir, "etc passwd")},
}

if runtime.GOOS == "windows" {
tc = append(tc, []input{
{dir, "d:\\etc\\test", filepath.Join(dir, "etc", "test")},
}...)
}

for _, test := range tc {
got, err := SecureJoin(test.root, test.unsafe)
if err != nil {
t.Errorf("securejoin(%q, %q): unexpected error: %v", test.root, test.unsafe, err)
continue
}
if got != expected {
t.Errorf("securejoin(%q, %q): expected %q, got %q", test.root, test.unsafe, expected, got)
continue
if got != test.expected {
t.Errorf("securejoin(%q, %q): expected %q, got %q", test.root, test.unsafe, test.expected, got)
}
}
}
Expand All @@ -130,10 +143,7 @@ func TestNonLexical(t *testing.T) {
symlink(t, "/../cousinparent/cousin", filepath.Join(dir, "subdir", "link2"))
symlink(t, "/../../../../../../../../../../../../../../../../cousinparent/cousin", filepath.Join(dir, "subdir", "link3"))

for _, test := range []struct {
root, unsafe string
expected string
}{
for _, test := range []input{
{dir, "subdir", filepath.Join(dir, "subdir")},
{dir, "subdir/link/test", filepath.Join(dir, "cousinparent", "cousin", "test")},
{dir, "subdir/link2/test", filepath.Join(dir, "cousinparent", "cousin", "test")},
Expand Down Expand Up @@ -188,7 +198,7 @@ func TestSymlinkLoop(t *testing.T) {
} {
got, err := SecureJoin(test.root, test.unsafe)
if !errors.Is(err, syscall.ELOOP) {
t.Errorf("securejoin(%q, %q): expected ELOOP, got %v & %q", test.root, test.unsafe, err, got)
t.Errorf("securejoin(%q, %q): expected ELOOP, got %q & %v", test.root, test.unsafe, got, err)
continue
}
}
Expand Down Expand Up @@ -275,10 +285,7 @@ func TestSecureJoinVFS(t *testing.T) {
symlink(t, "/../cousinparent/cousin", filepath.Join(dir, "subdir", "link2"))
symlink(t, "/../../../../../../../../../../../../../../../../cousinparent/cousin", filepath.Join(dir, "subdir", "link3"))

for _, test := range []struct {
root, unsafe string
expected string
}{
for _, test := range []input{
{dir, "subdir", filepath.Join(dir, "subdir")},
{dir, "subdir/link/test", filepath.Join(dir, "cousinparent", "cousin", "test")},
{dir, "subdir/link2/test", filepath.Join(dir, "cousinparent", "cousin", "test")},
Expand Down
Loading