Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/skaffold/docker/dockertest.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ func (f *fakeImageFetcher) fetch(_ context.Context, image string, _ Config) (*v1
},
},
}, nil
case "parentpath:onbuild":
return &v1.ConfigFile{
Config: v1.Config{
OnBuild: []string{
"COPY ../secret.txt /secret.txt",
},
},
}, nil
case "library/ruby:2.3.0":
return nil, fmt.Errorf("retrieving image \"library/ruby:2.3.0\": unsupported MediaType: \"application/vnd.docker.distribution.manifest.v1+prettyjws\", see https://github.com/google/go-containerregistry/issues/377")
}
Expand Down
42 changes: 40 additions & 2 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,13 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e
matchesOne := false

for _, p := range cpCmd.srcs {
path := filepath.Join(workspace, p)
path, relPath, err := resolveCopySourceInWorkspace(workspace, p)
if err != nil {
return nil, err
}

if _, err := os.Stat(path); err == nil {
fts = append(fts, FromTo{From: filepath.Clean(p), To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
fts = append(fts, FromTo{From: relPath, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
matchesOne = true
continue
}
Expand All @@ -232,6 +236,9 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e
if err != nil {
return nil, fmt.Errorf("getting relative path of %s", f)
}
if !isRelativePathInWorkspace(rel) {
return nil, fmt.Errorf("copy source %q resolves outside build context %q", p, workspace)
}

fts = append(fts, FromTo{From: rel, To: cpCmd.dest, ToIsDir: cpCmd.destIsDir, StartLine: cpCmd.startLine, EndLine: cpCmd.endLine})
}
Expand All @@ -248,6 +255,37 @@ func expandSrcGlobPatterns(workspace string, cpCmds []*copyCommand) ([]FromTo, e
return fts, nil
}

func resolveCopySourceInWorkspace(workspace, src string) (string, string, error) {
src = filepath.Clean(src)
if filepath.IsAbs(src) {
return "", "", fmt.Errorf("copy source %q resolves outside build context %q", src, workspace)
}

workspaceAbs, err := filepath.Abs(workspace)
if err != nil {
return "", "", fmt.Errorf("resolving build context %q: %w", workspace, err)
}
Comment thread
fsabiu marked this conversation as resolved.
Outdated

sourceAbs, err := filepath.Abs(filepath.Join(workspace, src))
if err != nil {
return "", "", fmt.Errorf("resolving copy source %q: %w", src, err)
}

rel, err := filepath.Rel(workspaceAbs, sourceAbs)
if err != nil {
return "", "", fmt.Errorf("getting relative path of %s", sourceAbs)
}
if !isRelativePathInWorkspace(rel) {
return "", "", fmt.Errorf("copy source %q resolves outside build context %q", src, workspace)
}

return filepath.Join(workspace, src), rel, nil
}

func isRelativePathInWorkspace(rel string) bool {
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) && !filepath.IsAbs(rel)
}

func extractCopyCommands(ctx context.Context, nodes []*parser.Node, onlyLastImage bool, cfg Config) ([]*copyCommand, error) {
stages := map[string]bool{
"scratch": true,
Expand Down
37 changes: 37 additions & 0 deletions pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,43 @@ func TestReadCopyCmdsFromDockerfile(t *testing.T) {
}
}

func TestReadCopyCmdsFromDockerfileRejectsSourcesOutsideWorkspace(t *testing.T) {
tests := []struct {
description string
dockerfile string
}{
{
description: "direct copy parent path",
dockerfile: "FROM nginx\nCOPY ../secret.txt /secret.txt",
},
{
description: "onbuild copy parent path",
dockerfile: "FROM parentpath:onbuild",
},
}

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
imageFetcher := fakeImageFetcher{}
t.Override(&RetrieveImage, imageFetcher.fetch)

tmp := t.NewTempDir()
tmp.Mkdir("workspace/project")
tmp.Write("workspace/secret.txt", "secret")

dockerfilePath := tmp.Path("workspace/project/Dockerfile")
err := os.WriteFile(dockerfilePath, []byte(test.dockerfile), 0644)
if err != nil {
t.Error(err)
}

cfg := mockConfig{mode: config.RunModes.Build}
_, err = ReadCopyCmdsFromDockerfile(context.Background(), false, dockerfilePath, tmp.Path("workspace/project"), make(map[string]*string), cfg)
t.CheckErrorContains("resolves outside build context", err)
})
}
}

func TestRemoveExtraBuildArgs(t *testing.T) {
tests := []struct {
description string
Expand Down