From 02ec2ae70fc2d52f0064cbf2d84b80d385e6a950 Mon Sep 17 00:00:00 2001 From: Joachim Krech <8290187+jkrech@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:29:29 +0100 Subject: [PATCH 1/2] Fix code scanning alert no. 49: Arbitrary file access during archive extraction ("Zip Slip") Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- cmd/installer/pack.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/installer/pack.go b/cmd/installer/pack.go index fecc0cf..8104075 100644 --- a/cmd/installer/pack.go +++ b/cmd/installer/pack.go @@ -166,6 +166,11 @@ func (p *PackType) validate() error { p.Subfolder = filepath.Dir(file.Name) } + // Ensure the file path does not contain ".." + if strings.Contains(file.Name, "..") { + return fmt.Errorf("invalid file path: %s", file.Name) + } + // Read pack's pdsc tmpPdscFileName := filepath.Join(os.TempDir(), utils.RandStringBytes(10)) defer os.RemoveAll(tmpPdscFileName) From ab2e4e14d54c33911b4473ca45e120aa967b2ef1 Mon Sep 17 00:00:00 2001 From: Bernd-Gunter Nitzler Date: Mon, 18 Nov 2024 17:49:19 +0100 Subject: [PATCH 2/2] Added a test for .. in file name --- cmd/errors/errors.go | 1 + cmd/installer/pack.go | 7 ++++++- cmd/installer/root_pack_add_test.go | 19 ++++++++++++++++++ cmd/installer/root_test.go | 1 + .../PackWith.ParentDirectoryFiles.1.2.3.pack | Bin 0 -> 620 bytes 5 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 testdata/integration/PackWith.ParentDirectoryFiles.1.2.3.pack diff --git a/cmd/errors/errors.go b/cmd/errors/errors.go index 6324605..3d5d208 100644 --- a/cmd/errors/errors.go +++ b/cmd/errors/errors.go @@ -58,6 +58,7 @@ var ( ErrPathAlreadyExists = errors.New("path already exists") ErrCopyingEqualPaths = errors.New("failed copying files: source is the same as destination") ErrMovingEqualPaths = errors.New("failed moving files: source is the same as destination") + ErrInvalidFilePath = errors.New("invalid file path") // Cryptography errors ErrIntegrityCheckFailed = errors.New("checksum verification failed") diff --git a/cmd/installer/pack.go b/cmd/installer/pack.go index 8104075..c8a0f20 100644 --- a/cmd/installer/pack.go +++ b/cmd/installer/pack.go @@ -168,7 +168,8 @@ func (p *PackType) validate() error { // Ensure the file path does not contain ".." if strings.Contains(file.Name, "..") { - return fmt.Errorf("invalid file path: %s", file.Name) + log.Errorf("File \"%s\" invalid file path", file.Name) + return errs.ErrInvalidFilePath } // Read pack's pdsc @@ -203,6 +204,10 @@ func (p *PackType) validate() error { p.Pdsc.FileName = file.Name return nil + } else { + if strings.Contains(file.Name, "..") { + return errs.ErrInsecureZipFileName + } } } diff --git a/cmd/installer/root_pack_add_test.go b/cmd/installer/root_pack_add_test.go index a9549b9..0e58cce 100644 --- a/cmd/installer/root_pack_add_test.go +++ b/cmd/installer/root_pack_add_test.go @@ -311,6 +311,25 @@ func TestAddPack(t *testing.T) { assert.False(utils.FileExists(installer.Installation.PackIdx)) }) + t.Run("test installing a pack with .. in pdsc name", func(t *testing.T) { + localTestingDir := "test-add-pack-with-dot-dot-name" + assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot)) + installer.UnlockPackRoot() + installer.Installation.WebDir = filepath.Join(testDir, "public_index") + defer removePackRoot(localTestingDir) + + packPath := packWithParentDirectoryFiles + + err := installer.AddPack(packPath, !CheckEula, !ExtractEula, !ForceReinstall, !NoRequirements, Timeout) + + // Sanity check + assert.NotNil(err) + assert.Equal(errs.ErrInvalidFilePath, err) + + // Make sure pack.idx never got touched + assert.False(utils.FileExists(installer.Installation.PackIdx)) + }) + t.Run("test installing a pack with version not present in the pdsc file", func(t *testing.T) { localTestingDir := "test-add-pack-with-version-not-present-in-the-pdsc-file" assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot)) diff --git a/cmd/installer/root_test.go b/cmd/installer/root_test.go index 3adf918..25cfd00 100644 --- a/cmd/installer/root_test.go +++ b/cmd/installer/root_test.go @@ -240,6 +240,7 @@ var ( packWithMalformedURL = "http://:malformed-url*/TheVendor.PackName.1.2.3.pack" packWithoutPdscFileInside = filepath.Join(testDir, "PackWithout.PdscFileInside.1.2.3.pack") packWithTaintedCompressedFiles = filepath.Join(testDir, "PackWith.TaintedFiles.1.2.3.pack") + packWithParentDirectoryFiles = filepath.Join(testDir, "PackWith.ParentDirectoryFiles.1.2.3.pack") // Packs with packid names only publicRemotePackPackID = "TheVendor.PublicRemotePack" diff --git a/testdata/integration/PackWith.ParentDirectoryFiles.1.2.3.pack b/testdata/integration/PackWith.ParentDirectoryFiles.1.2.3.pack new file mode 100644 index 0000000000000000000000000000000000000000..6478d6e372d1a94d4f4350ce76d2641696634168 GIT binary patch literal 620 zcmWIWW@Zs#U|`^2SP)|vTylc?GN_6ufBGE=4bY}XO#!-?_aa7Z3#;~mJ|@itjesn=JpCPgIf-ZuKTSn zIC;m;cK;^EY_0SbfkL)uYyItp9}c%He!%JR7zCjTibuR1exG3sAsnW48K zrPwP=bitz3xI5`Tlf&)=?rfYFbJgd=>(|@-J)d@)9$fKk{<(<1yebt|fhl#20l