diff --git a/cli/context/store/store.go b/cli/context/store/store.go index b3cd4c54ed..5ad4d7759b 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -7,7 +7,6 @@ import ( "bytes" _ "crypto/sha256" // ensure ids can be computed "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -18,6 +17,7 @@ import ( "github.com/docker/docker/errdefs" digest "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) // Store provides a context store for easily remembering endpoints configuration @@ -295,6 +295,19 @@ func Import(name string, s Writer, reader io.Reader) error { } } +func isValidFilePath(p string) error { + if p != metaFile && !strings.HasPrefix(p, "tls/") { + return errors.New("unexpected context file") + } + if path.Clean(p) != p { + return errors.New("unexpected path format") + } + if strings.Contains(p, `\`) { + return errors.New(`unexpected '\' in path`) + } + return nil +} + func importTar(name string, s Writer, reader io.Reader) error { tr := tar.NewReader(&LimitedReader{R: reader, N: maxAllowedFileSizeToImport}) tlsData := ContextTLSData{ @@ -309,10 +322,13 @@ func importTar(name string, s Writer, reader io.Reader) error { if err != nil { return err } - if hdr.Typeflag == tar.TypeDir { + if hdr.Typeflag != tar.TypeReg { // skip this entry, only taking files into account continue } + if err := isValidFilePath(hdr.Name); err != nil { + return errors.Wrap(err, hdr.Name) + } if hdr.Name == metaFile { data, err := ioutil.ReadAll(tr) if err != nil { @@ -358,10 +374,13 @@ func importZip(name string, s Writer, reader io.Reader) error { var importedMetaFile bool for _, zf := range zr.File { fi := zf.FileInfo() - if fi.IsDir() { - // skip this entry, only taking files into account + if !fi.Mode().IsRegular() { + // skip this entry, only taking regular files into account continue } + if err := isValidFilePath(zf.Name); err != nil { + return errors.Wrap(err, zf.Name) + } if zf.Name == metaFile { f, err := zf.Open() if err != nil { diff --git a/cli/context/store/store_test.go b/cli/context/store/store_test.go index df506061e5..4b412652de 100644 --- a/cli/context/store/store_test.go +++ b/cli/context/store/store_test.go @@ -175,7 +175,7 @@ func TestImportTarInvalid(t *testing.T) { var r io.Reader = source s := New(testDir, testCfg) err = Import("tarInvalid", s, r) - assert.ErrorContains(t, err, "invalid context: no metadata found") + assert.ErrorContains(t, err, "unexpected context file") } func TestImportZip(t *testing.T) { @@ -254,5 +254,5 @@ func TestImportZipInvalid(t *testing.T) { var r io.Reader = source s := New(testDir, testCfg) err = Import("zipInvalid", s, r) - assert.ErrorContains(t, err, "invalid context: no metadata found") + assert.ErrorContains(t, err, "unexpected context file") } diff --git a/cli/context/store/storeconfig_test.go b/cli/context/store/storeconfig_test.go index 7f59e3f015..e7384fb071 100644 --- a/cli/context/store/storeconfig_test.go +++ b/cli/context/store/storeconfig_test.go @@ -29,3 +29,19 @@ func TestConfigModification(t *testing.T) { assert.Equal(t, &testEP2{}, cfgCopy.endpointTypes["ep1"]()) assert.Equal(t, &testEP3{}, cfgCopy.endpointTypes["ep2"]()) } + +func TestValidFilePaths(t *testing.T) { + paths := map[string]bool{ + "tls/_/../../something": false, + "tls/../../something": false, + "../../something": false, + "/tls/absolute/unix/path": false, + `C:\tls\absolute\windows\path`: false, + "C:/tls/absolute/windows/path": false, + "tls/kubernetes/key.pem": true, + } + for p, expectedValid := range paths { + err := isValidFilePath(p) + assert.Equal(t, err == nil, expectedValid, "%q should report valid as: %v", p, expectedValid) + } +}