Skip to content

Commit

Permalink
remove isAbs since .Join covers this check
Browse files Browse the repository at this point in the history
  • Loading branch information
dduzgun-security committed Jan 27, 2025
1 parent 7093255 commit b0e7c40
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
9 changes: 2 additions & 7 deletions internal/unpackinfo/unpackinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,14 @@ func NewUnpackInfo(dst string, header *tar.Header) (UnpackInfo, error) {

// Clean the destination path
dst = filepath.Clean(dst)

// Get rid of absolute paths.
path := header.Name
if filepath.IsAbs(path) {
path = path[1:]
}
path := filepath.Clean(header.Name)

path = filepath.Join(dst, path)
target := filepath.Clean(path)

// Check for path traversal by ensuring the target is within the destination
rel, err := filepath.Rel(dst, target)
if err != nil || strings.HasPrefix(rel, "..") || strings.Contains(dst, "..") || strings.Contains(target, "..") || filepath.IsAbs(rel) {
if err != nil || strings.HasPrefix(rel, "..") || strings.Contains(dst, "..") || strings.Contains(path, "..") || strings.Contains(target, "..") {
return UnpackInfo{}, errors.New("invalid filename, traversal with \"..\" outside of current directory")
}

Expand Down
22 changes: 19 additions & 3 deletions internal/unpackinfo/unpackinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ func TestNewUnpackInfo(t *testing.T) {
}
})
t.Run("destination starting with ./", func(t *testing.T) {
outsideDst := filepath.Join("./" + t.TempDir())
dst := t.TempDir()
outsideDst := "./" + dst
result, err := NewUnpackInfo(outsideDst, &tar.Header{
Name: "foo.txt",
Typeflag: tar.TypeSymlink,
Expand All @@ -163,8 +164,23 @@ func TestNewUnpackInfo(t *testing.T) {

t.Run("destination starting with ./ followed with ../", func(t *testing.T) {
dst := t.TempDir()
outsideDst := filepath.Join("./../../" + dst)
_, err := NewUnpackInfo(outsideDst, &tar.Header{
_, err := NewUnpackInfo("./../../"+dst, &tar.Header{
Name: "foo.txt",
Typeflag: tar.TypeSymlink,
})

if err == nil {
t.Fatal("expected error, got nil")
}

expected := "traversal with \"..\" outside of current"
if !strings.Contains(err.Error(), expected) {
t.Fatalf("expected error to contain %q, got %q", expected, err)
}
})
t.Run("destination followed with ../", func(t *testing.T) {
dst := t.TempDir()
_, err := NewUnpackInfo(dst+"../../foo", &tar.Header{
Name: "foo.txt",
Typeflag: tar.TypeSymlink,
})
Expand Down

0 comments on commit b0e7c40

Please sign in to comment.