From 11fbaaae9ae5c7e41a7994597d4155cc2b86faad Mon Sep 17 00:00:00 2001 From: greatroar <@> Date: Sat, 3 Oct 2020 13:27:23 +0200 Subject: [PATCH] Sanitize environment before starting backend processes (rclone, ssh) The restic security model includes full trust of the local machine, so this should not fix any actual security problems, but it's better to be safe than sorry. Fixes #2192. --- internal/backend/foreground.go | 26 ++++++++++++++++++ internal/backend/foreground_solaris.go | 5 +--- internal/backend/foreground_test.go | 38 ++++++++++++++++++++++++++ internal/backend/foreground_unix.go | 5 +--- internal/backend/foreground_windows.go | 5 +--- 5 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 internal/backend/foreground.go create mode 100644 internal/backend/foreground_test.go diff --git a/internal/backend/foreground.go b/internal/backend/foreground.go new file mode 100644 index 000000000..7291dc8d6 --- /dev/null +++ b/internal/backend/foreground.go @@ -0,0 +1,26 @@ +package backend + +import ( + "os" + "os/exec" + "strings" +) + +// StartForeground runs cmd in the foreground, by temporarily switching to the +// new process group created for cmd. The returned function `bg` switches back +// to the previous process group. +// +// The command's environment has all RESTIC_* variables removed. +func StartForeground(cmd *exec.Cmd) (bg func() error, err error) { + env := os.Environ() // Returns a copy that we can modify. + + cmd.Env = env[:0] + for _, kv := range env { + if strings.HasPrefix(kv, "RESTIC_") { + continue + } + cmd.Env = append(cmd.Env, kv) + } + + return startForeground(cmd) +} diff --git a/internal/backend/foreground_solaris.go b/internal/backend/foreground_solaris.go index 501f9c1a1..8b963db21 100644 --- a/internal/backend/foreground_solaris.go +++ b/internal/backend/foreground_solaris.go @@ -7,10 +7,7 @@ import ( "github.com/restic/restic/internal/errors" ) -// StartForeground runs cmd in the foreground, by temporarily switching to the -// new process group created for cmd. The returned function `bg` switches back -// to the previous process group. -func StartForeground(cmd *exec.Cmd) (bg func() error, err error) { +func startForeground(cmd *exec.Cmd) (bg func() error, err error) { // run the command in it's own process group so that SIGINT // is not sent to it. cmd.SysProcAttr = &syscall.SysProcAttr{ diff --git a/internal/backend/foreground_test.go b/internal/backend/foreground_test.go new file mode 100644 index 000000000..34c55d1f3 --- /dev/null +++ b/internal/backend/foreground_test.go @@ -0,0 +1,38 @@ +// +build !windows + +package backend_test + +import ( + "bufio" + "os" + "os/exec" + "strings" + "testing" + + "github.com/restic/restic/internal/backend" + rtest "github.com/restic/restic/internal/test" +) + +func TestForeground(t *testing.T) { + err := os.Setenv("RESTIC_PASSWORD", "supersecret") + rtest.OK(t, err) + + cmd := exec.Command("env") + stdout, err := cmd.StdoutPipe() + rtest.OK(t, err) + + bg, err := backend.StartForeground(cmd) + rtest.OK(t, err) + defer cmd.Wait() + + err = bg() + rtest.OK(t, err) + + sc := bufio.NewScanner(stdout) + for sc.Scan() { + if strings.HasPrefix(sc.Text(), "RESTIC_PASSWORD=") { + t.Error("subprocess got to see the password") + } + } + rtest.OK(t, err) +} diff --git a/internal/backend/foreground_unix.go b/internal/backend/foreground_unix.go index 1662a0250..fe5b95ffa 100644 --- a/internal/backend/foreground_unix.go +++ b/internal/backend/foreground_unix.go @@ -24,10 +24,7 @@ func tcsetpgrp(fd int, pid int) error { return errno } -// StartForeground runs cmd in the foreground, by temporarily switching to the -// new process group created for cmd. The returned function `bg` switches back -// to the previous process group. -func StartForeground(cmd *exec.Cmd) (bg func() error, err error) { +func startForeground(cmd *exec.Cmd) (bg func() error, err error) { // open the TTY, we need the file descriptor tty, err := os.OpenFile("/dev/tty", os.O_RDWR, 0) if err != nil { diff --git a/internal/backend/foreground_windows.go b/internal/backend/foreground_windows.go index 281d5f3f8..1c3d1a077 100644 --- a/internal/backend/foreground_windows.go +++ b/internal/backend/foreground_windows.go @@ -6,10 +6,7 @@ import ( "github.com/restic/restic/internal/errors" ) -// StartForeground runs cmd in the foreground, by temporarily switching to the -// new process group created for cmd. The returned function `bg` switches back -// to the previous process group. -func StartForeground(cmd *exec.Cmd) (bg func() error, err error) { +func startForeground(cmd *exec.Cmd) (bg func() error, err error) { // just start the process and hope for the best err = cmd.Start() if err != nil {