diff options
author | Martin Polden <mpolden@mpolden.no> | 2022-09-20 14:47:09 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-09-20 14:47:09 +0200 |
commit | f0ff25ce823219e4454e03328aef0a2c0c81302a (patch) | |
tree | 07e6b76f906933398bb7fc4f465ce1b3cdb1fde4 | |
parent | 8ea0eb626e7c9403167175c81282c6aa315c80c4 (diff) | |
parent | d2b8573f68db1114eaf86cbd9544a9974f9b1b2b (diff) |
Merge pull request #24140 from vespa-engine/mpolden/validate-zip-entry-path
Validate zip entry path when unpacking
-rw-r--r-- | client/go/cmd/prod_test.go | 24 | ||||
-rw-r--r-- | client/go/cmd/testdata/applications/withInvalidEntries/target/application-test.zip | bin | 0 -> 261 bytes | |||
-rw-r--r-- | client/go/cmd/testdata/applications/withInvalidEntries/target/application.zip | bin | 0 -> 261 bytes | |||
-rw-r--r-- | client/go/vespa/application.go | 20 |
4 files changed, 42 insertions, 2 deletions
diff --git a/client/go/cmd/prod_test.go b/client/go/cmd/prod_test.go index 3c1799701a3..eff06cf860e 100644 --- a/client/go/cmd/prod_test.go +++ b/client/go/cmd/prod_test.go @@ -217,6 +217,30 @@ func TestProdSubmitWithJava(t *testing.T) { assert.Contains(t, stdout.String(), "See https://console.vespa-cloud.com/tenant/t1/application/a1/prod/deployment for deployment progress") } +func TestProdSubmitInvalidZip(t *testing.T) { + pkgDir := filepath.Join(t.TempDir(), "app") + createApplication(t, pkgDir, true) + + httpClient := &mock.HTTPClient{} + httpClient.NextResponseString(200, `ok`) + cli, _, stderr := newTestCLI(t, "CI=true") + cli.httpClient = httpClient + assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1")) + assert.Nil(t, cli.Run("config", "set", "target", "cloud")) + assert.Nil(t, cli.Run("auth", "api-key")) + assert.Nil(t, cli.Run("auth", "cert", pkgDir)) + + // Copy an invalid application package containing relative file names + testAppDir := filepath.Join("testdata", "applications", "withInvalidEntries", "target") + zipFile := filepath.Join(testAppDir, "application.zip") + copyFile(t, filepath.Join(pkgDir, "target", "application.zip"), zipFile) + testZipFile := filepath.Join(testAppDir, "application-test.zip") + copyFile(t, filepath.Join(pkgDir, "target", "application-test.zip"), testZipFile) + + assert.NotNil(t, cli.Run("prod", "submit", pkgDir)) + assert.Equal(t, "Error: found invalid path inside zip: ../../../../../../../tmp/foo\n", stderr.String()) +} + func copyFile(t *testing.T, dstFilename, srcFilename string) { dst, err := os.Create(dstFilename) if err != nil { diff --git a/client/go/cmd/testdata/applications/withInvalidEntries/target/application-test.zip b/client/go/cmd/testdata/applications/withInvalidEntries/target/application-test.zip Binary files differnew file mode 100644 index 00000000000..3e4e8c23f9b --- /dev/null +++ b/client/go/cmd/testdata/applications/withInvalidEntries/target/application-test.zip diff --git a/client/go/cmd/testdata/applications/withInvalidEntries/target/application.zip b/client/go/cmd/testdata/applications/withInvalidEntries/target/application.zip Binary files differnew file mode 100644 index 00000000000..3e4e8c23f9b --- /dev/null +++ b/client/go/cmd/testdata/applications/withInvalidEntries/target/application.zip diff --git a/client/go/vespa/application.go b/client/go/vespa/application.go index 06c643c2b3f..2f19c00b3f2 100644 --- a/client/go/vespa/application.go +++ b/client/go/vespa/application.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/vespa-engine/vespa/client/go/util" ) @@ -165,6 +166,9 @@ func (ap *ApplicationPackage) Unzip(test bool) (string, error) { } defer f.Close() for _, f := range f.File { + if !validPath(f.Name) { + return "", fmt.Errorf("found invalid path inside zip: %s", f.Name) + } dst := filepath.Join(tmp, f.Name) if f.FileInfo().IsDir() { if err := os.Mkdir(dst, f.FileInfo().Mode()); err != nil { @@ -173,14 +177,26 @@ func (ap *ApplicationPackage) Unzip(test bool) (string, error) { continue } if err := copyFile(f, dst); err != nil { - return "", fmt.Errorf("copyFile: %w", err) + return "", fmt.Errorf("copying %s to %s failed: %w", f.Name, dst, err) } - } cleanTemp = false return tmp, nil } +func validPath(path string) bool { + path = strings.TrimSuffix(path, "/") + if filepath.Clean(path) != path { + return false + } + for _, part := range strings.Split(path, "/") { + if part == ".." { + return false + } + } + return true +} + func copyFile(src *zip.File, dst string) error { from, err := src.Open() if err != nil { |