From 7052f297326972b243647d5423a6519f7f903d08 Mon Sep 17 00:00:00 2001 From: Christopher Phillips Date: Thu, 29 Feb 2024 11:52:32 -0500 Subject: [PATCH 1/3] fix: update tar traversal to respect current director entry Signed-off-by: Christopher Phillips --- pkg/file/tarutil.go | 8 +++++++- pkg/file/tarutil_test.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index 54bf0a87..6a8b10ee 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -147,7 +147,9 @@ func (v tarVisitor) visit(entry TarFileEntry) error { target := filepath.Join(v.destination, entry.Header.Name) // we should not allow for any destination path to be outside of where we are unarchiving to - if !strings.HasPrefix(target, v.destination+string(os.PathSeparator)) { + // "." is a special case that we allow (it is the root of the unarchived content)a + test := v.destination + string(os.PathSeparator) + if !strings.HasPrefix(target, test) && entry.Header.Name != "." { return fmt.Errorf("potential path traversal attack with entry: %q", entry.Header.Name) } @@ -157,6 +159,10 @@ func (v tarVisitor) visit(entry TarFileEntry) error { log.WithFields("path", entry.Header.Name).Trace("skipping symlink/link entry in image tar") case tar.TypeDir: + // we don't need to do anything for directories, they are created as needed + if entry.Header.Name == "." { + return nil + } if _, err := v.fs.Stat(target); err != nil { if err := v.fs.MkdirAll(target, 0755); err != nil { return err diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go index 721de817..8320986b 100644 --- a/pkg/file/tarutil_test.go +++ b/pkg/file/tarutil_test.go @@ -279,6 +279,20 @@ func Test_tarVisitor_visit(t *testing.T) { }, wantErr: require.Error, }, + { + name: "local . index is not a traversal error and should skip", + entry: TarFileEntry{ + Sequence: 0, + Header: tar.Header{ + Typeflag: tar.TypeDir, + Name: ".", + Linkname: "", + Size: 2, + }, + Reader: strings.NewReader("hi"), + }, + assertFs: []func(t testing.TB, fs afero.Fs){}, + }, { name: "regular file with possible path traversal errors out (same prefix)", entry: TarFileEntry{ From 017568649b60f2e89bd55d3329c72b21c878d94c Mon Sep 17 00:00:00 2001 From: Christopher Phillips Date: Thu, 29 Feb 2024 12:08:26 -0500 Subject: [PATCH 2/3] fix: update variable name Signed-off-by: Christopher Phillips --- pkg/file/tarutil.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index 6a8b10ee..d890937a 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -148,8 +148,8 @@ func (v tarVisitor) visit(entry TarFileEntry) error { // we should not allow for any destination path to be outside of where we are unarchiving to // "." is a special case that we allow (it is the root of the unarchived content)a - test := v.destination + string(os.PathSeparator) - if !strings.HasPrefix(target, test) && entry.Header.Name != "." { + withinDir := v.destination + string(os.PathSeparator) + if !strings.HasPrefix(target, withinDir) && entry.Header.Name != "." { return fmt.Errorf("potential path traversal attack with entry: %q", entry.Header.Name) } From 1b7c7d5825a8a861e927717eba39a12bdf75dca9 Mon Sep 17 00:00:00 2001 From: Christopher Phillips Date: Thu, 29 Feb 2024 12:19:32 -0500 Subject: [PATCH 3/3] fix: extra comment Signed-off-by: Christopher Phillips --- pkg/file/tarutil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go index d890937a..01f19f1b 100644 --- a/pkg/file/tarutil.go +++ b/pkg/file/tarutil.go @@ -147,7 +147,7 @@ func (v tarVisitor) visit(entry TarFileEntry) error { target := filepath.Join(v.destination, entry.Header.Name) // we should not allow for any destination path to be outside of where we are unarchiving to - // "." is a special case that we allow (it is the root of the unarchived content)a + // "." is a special case that we allow (it is the root of the unarchived content) withinDir := v.destination + string(os.PathSeparator) if !strings.HasPrefix(target, withinDir) && entry.Header.Name != "." { return fmt.Errorf("potential path traversal attack with entry: %q", entry.Header.Name)