diff --git a/internal/cmd/io_binary.go b/internal/cmd/io_binary.go index d9e0f9749a..3647b66aae 100644 --- a/internal/cmd/io_binary.go +++ b/internal/cmd/io_binary.go @@ -28,6 +28,8 @@ const ( binaryCmdStartTimeout = 10 * time.Second ) +var ErrUnsafePath = errors.New("path is unsafe") + // NewBinaryIO runs a custom binary process for pluggable shim logging driver. // // Container's IO will be redirected to the logging driver via named pipes, which are @@ -122,14 +124,19 @@ func NewBinaryIO(ctx context.Context, id string, uri *url.URL) (_ UpstreamIO, er } // sanitizePath parses the URL object and returns a clean path to the logging driver -func sanitizePath(uri *url.URL) string { +func sanitizePath(uri *url.URL) (string, error) { path := filepath.Clean(uri.Path) + // avoid UNC paths (e.g. `\\server\share\`) + if strings.HasPrefix(path, `\\`) { + return "", ErrUnsafePath + } + if strings.Contains(path, `:\`) { - return strings.TrimPrefix(path, "\\") + return strings.TrimPrefix(path, "\\"), nil } - return path + return path, nil } func newBinaryCmd(ctx context.Context, uri *url.URL, envs []string) (*exec.Cmd, error) { @@ -145,7 +152,10 @@ func newBinaryCmd(ctx context.Context, uri *url.URL, envs []string) (*exec.Cmd, } } - execPath := sanitizePath(uri) + execPath, err := sanitizePath(uri) + if err != nil { + return nil, err + } cmd := exec.CommandContext(ctx, execPath, args...) cmd.Env = append(cmd.Env, envs...) diff --git a/internal/cmd/io_binary_test.go b/internal/cmd/io_binary_test.go index fcb00a511b..a930a8d5ac 100644 --- a/internal/cmd/io_binary_test.go +++ b/internal/cmd/io_binary_test.go @@ -4,6 +4,7 @@ package cmd import ( "context" + "errors" "net/url" "testing" ) @@ -24,11 +25,6 @@ func Test_newBinaryCmd_Key_Value_Pair(t *testing.T) { urlString: "binary:///executable?-key=value", expected: `\executable -key value`, }, - { - name: "Path_With_Back_Slashes", - urlString: `binary:///\executable?-key=value`, - expected: `\executable -key value`, - }, { name: "Clean_Path_With_Dots_And_Multiple_Fwd_Slashes", urlString: "binary:///../path/to///to/../executable", @@ -70,6 +66,45 @@ func Test_newBinaryCmd_Key_Value_Pair(t *testing.T) { } } +func Test_newBinaryCmd_Unsafe_Path(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + type config struct { + name string + urlString string + expectedError error + } + + for _, cfg := range []*config{ + { + name: "UNC_Path_With_Back_Slashes", + urlString: `binary:///\server\share\executable`, + expectedError: ErrUnsafePath, + }, + { + name: "UNC_Path_With_Forward_Slashes", + urlString: `binary:////server/share/executable`, + expectedError: ErrUnsafePath, + }, + } { + t.Run(cfg.name, func(t *testing.T) { + u, err := url.Parse(cfg.urlString) + if err != nil { + t.Fatalf("failed to parse url: %s", cfg.urlString) + } + + _, err = newBinaryCmd(ctx, u, nil) + if err == nil { + t.Fatalf("no error was returned") + } + if !errors.Is(err, cfg.expectedError) { + t.Fatalf("expected error: %s, actual: %s", cfg.expectedError, err) + } + }) + } +} + func Test_newBinaryCmd_Empty_Path(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel()