context: Ensure import paths are valid

Signed-off-by: Chris Crone <[email protected]>
docker · Jan 28, 2021 · 6f49197 · 6f49197
1 parent 2291f61
commit 6f49197
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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)
+	}
+}