Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent overwriting irregular files (cp, save, export commands) #1515

Merged
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 9 additions & 0 deletions cli/command/container/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ func copyFromContainer(ctx context.Context, dockerCli command.Cli, copyConfig cp
}
}

if err := command.ValidateOutputPath(dstPath); err != nil {
return err
}

client := dockerCli.Client()
// if client requests to follow symbol link, then must decide target file to be copied
var rebaseName string
Expand Down Expand Up @@ -209,6 +213,11 @@ func copyToContainer(ctx context.Context, dockerCli command.Cli, copyConfig cpCo
dstStat, err = client.ContainerStatPath(ctx, copyConfig.container, linkTarget)
}

// Validate the destination path
if err := command.ValidateOutputPathFileMode(dstStat.Mode); err != nil {
return errors.Wrapf(err, `destination "%s:%s" must be a directory or a regular file`, copyConfig.container, dstPath)
}

// Ignore any error and assume that the parent directory of the destination
// path exists, in which case the copy may still succeed. If there is any
// type of conflict (e.g., non-directory overwriting an existing directory
Expand Down
9 changes: 9 additions & 0 deletions cli/command/container/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,12 @@ func TestSplitCpArg(t *testing.T) {
})
}
}

func TestRunCopyFromContainerToFilesystemIrregularDestination(t *testing.T) {
options := copyOptions{source: "container:/dev/null", destination: "/dev/random"}
cli := test.NewFakeCli(nil)
err := runCopy(cli, options)
assert.Assert(t, err != nil)
expected := `"/dev/random" must be a directory or a regular file`
assert.ErrorContains(t, err, expected)
}
4 changes: 4 additions & 0 deletions cli/command/container/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ func runExport(dockerCli command.Cli, opts exportOptions) error {
return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
}

if err := command.ValidateOutputPath(opts.output); err != nil {
return errors.Wrap(err, "failed to export container")
}

clnt := dockerCli.Client()

responseBody, err := clnt.ContainerExport(context.Background(), opts.container)
Expand Down
16 changes: 16 additions & 0 deletions cli/command/container/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,19 @@ func TestContainerExportOutputToFile(t *testing.T) {

assert.Assert(t, fs.Equal(dir.Path(), expected))
}

func TestContainerExportOutputToIrregularFile(t *testing.T) {
cli := test.NewFakeCli(&fakeClient{
containerExportFunc: func(container string) (io.ReadCloser, error) {
return ioutil.NopCloser(strings.NewReader("foo")), nil
},
})
cmd := NewExportCommand(cli)
cmd.SetOutput(ioutil.Discard)
cmd.SetArgs([]string{"-o", "/dev/random", "container"})

err := cmd.Execute()
assert.Assert(t, err != nil)
expected := `"/dev/random" must be a directory or a regular file`
assert.ErrorContains(t, err, expected)
}
14 changes: 1 addition & 13 deletions cli/command/image/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package image
import (
"context"
"io"
"os"
"path/filepath"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -44,7 +42,7 @@ func RunSave(dockerCli command.Cli, opts saveOptions) error {
return errors.New("cowardly refusing to save to a terminal. Use the -o flag or redirect")
}

if err := validateOutputPath(opts.output); err != nil {
if err := command.ValidateOutputPath(opts.output); err != nil {
return errors.Wrap(err, "failed to save image")
}

Expand All @@ -61,13 +59,3 @@ func RunSave(dockerCli command.Cli, opts saveOptions) error {

return command.CopyToFile(opts.output, responseBody)
}

func validateOutputPath(path string) error {
dir := filepath.Dir(path)
if dir != "" && dir != "." {
if _, err := os.Stat(dir); os.IsNotExist(err) {
return errors.Errorf("unable to validate output path: directory %q does not exist", dir)
}
}
return nil
}
7 changes: 6 additions & 1 deletion cli/command/image/save_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ func TestNewSaveCommandErrors(t *testing.T) {
{
name: "output directory does not exist",
args: []string{"-o", "fakedir/out.tar", "arg1"},
expectedError: "failed to save image: unable to validate output path: directory \"fakedir\" does not exist",
expectedError: "failed to save image: invalid output path: directory \"fakedir\" does not exist",
},
{
name: "output file is irregular",
args: []string{"-o", "/dev/null", "arg1"},
expectedError: "failed to save image: invalid output path: \"/dev/null\" must be a directory or a regular file",
},
}
for _, tc := range testCases {
Expand Down
35 changes: 35 additions & 0 deletions cli/command/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/pkg/system"
"github.com/pkg/errors"
"github.com/spf13/pflag"
)

Expand Down Expand Up @@ -125,3 +126,37 @@ func AddPlatformFlag(flags *pflag.FlagSet, target *string) {
flags.SetAnnotation("platform", "version", []string{"1.32"})
flags.SetAnnotation("platform", "experimental", nil)
}

// ValidateOutputPath validates the output paths of the `export` and `save` commands.
func ValidateOutputPath(path string) error {
dir := filepath.Dir(path)
if dir != "" && dir != "." {
if _, err := os.Stat(dir); os.IsNotExist(err) {
return errors.Errorf("invalid output path: directory %q does not exist", dir)
}
}
// check whether `path` points to a regular file
// (if the path exists and doesn't point to a directory)
if fileInfo, err := os.Stat(path); !os.IsNotExist(err) {
if fileInfo.Mode().IsDir() || fileInfo.Mode().IsRegular() {
return nil
}

if err := ValidateOutputPathFileMode(fileInfo.Mode()); err != nil {
return errors.Wrapf(err, fmt.Sprintf("invalid output path: %q must be a directory or a regular file", path))
}
}
return nil
}

// ValidateOutputPathFileMode validates the output paths of the `cp` command and serves as a
// helper to `ValidateOutputPath`
func ValidateOutputPathFileMode(fileMode os.FileMode) error {
switch {
case fileMode&os.ModeDevice != 0:
return errors.New("got a device")
case fileMode&os.ModeIrregular != 0:
return errors.New("got an irregular file")
}
return nil
}