From 10c4ffc6b888eee8f2134a7009a0db1bc393e17b Mon Sep 17 00:00:00 2001
From: "brian m. carlson" <bk2204@github.com>
Date: Mon, 21 Dec 2020 21:08:17 +0000
Subject: [PATCH] Use subprocess for invoking all commands

The fix for CVE-2020-27955 was incomplete because we did not consider
places outside of the subprocess code that invoke binaries.  As a
result, there are still some places where an attacker can execute
arbitrary code by placing a malicious binary in the repository.

To make sure we've covered all the bases, let's just use the subprocess
code for executing all programs, which means that they'll be secure.  As
of this commit, all users of exec.Command are in test code or the
subprocess code itself.
---
 commands/commands.go |  3 +--
 creds/creds.go       |  5 +++--
 lfs/extension.go     |  6 +++---
 lfshttp/ssh.go       |  3 +--
 t/t-path.sh          | 38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/commands/commands.go b/commands/commands.go
index fdd6b2a418..04e04b6b6c 100644
--- a/commands/commands.go
+++ b/commands/commands.go
@@ -7,7 +7,6 @@ import (
 	"log"
 	"net"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -282,7 +281,7 @@ func PipeMediaCommand(name string, args ...string) error {
 }
 
 func PipeCommand(name string, args ...string) error {
-	cmd := exec.Command(name, args...)
+	cmd := subprocess.ExecCommand(name, args...)
 	cmd.Stdin = os.Stdin
 	cmd.Stderr = os.Stderr
 	cmd.Stdout = os.Stdout
diff --git a/creds/creds.go b/creds/creds.go
index 2de895fece..393a10d1fb 100644
--- a/creds/creds.go
+++ b/creds/creds.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/git-lfs/git-lfs/config"
 	"github.com/git-lfs/git-lfs/errors"
+	"github.com/git-lfs/git-lfs/subprocess"
 	"github.com/rubyist/tracerx"
 )
 
@@ -232,7 +233,7 @@ func (a *AskPassCredentialHelper) getFromProgram(valueType credValueType, u *url
 
 	// 'cmd' will run the GIT_ASKPASS (or core.askpass) command prompting
 	// for the desired valueType (`Username` or `Password`)
-	cmd := exec.Command(a.Program, a.args(fmt.Sprintf("%s for %q", valueString, u))...)
+	cmd := subprocess.ExecCommand(a.Program, a.args(fmt.Sprintf("%s for %q", valueString, u))...)
 	cmd.Stderr = &err
 	cmd.Stdout = &value
 
@@ -292,7 +293,7 @@ func (h *commandCredentialHelper) Approve(creds Creds) error {
 
 func (h *commandCredentialHelper) exec(subcommand string, input Creds) (Creds, error) {
 	output := new(bytes.Buffer)
-	cmd := exec.Command("git", "credential", subcommand)
+	cmd := subprocess.ExecCommand("git", "credential", subcommand)
 	cmd.Stdin = bufferCreds(input)
 	cmd.Stdout = output
 	/*
diff --git a/lfs/extension.go b/lfs/extension.go
index 2b04b9e45b..3fab1303ba 100644
--- a/lfs/extension.go
+++ b/lfs/extension.go
@@ -8,10 +8,10 @@ import (
 	"hash"
 	"io"
 	"os"
-	"os/exec"
 	"strings"
 
 	"github.com/git-lfs/git-lfs/config"
+	"github.com/git-lfs/git-lfs/subprocess"
 )
 
 type pipeRequest struct {
@@ -33,7 +33,7 @@ type pipeExtResult struct {
 }
 
 type extCommand struct {
-	cmd    *exec.Cmd
+	cmd    *subprocess.Cmd
 	out    io.WriteCloser
 	err    *bytes.Buffer
 	hasher hash.Hash
@@ -75,7 +75,7 @@ func pipeExtensions(cfg *config.Configuration, request *pipeRequest) (response p
 			arg := strings.Replace(value, "%f", request.fileName, -1)
 			args = append(args, arg)
 		}
-		cmd := exec.Command(name, args...)
+		cmd := subprocess.ExecCommand(name, args...)
 		ec := &extCommand{cmd: cmd, result: &pipeExtResult{name: e.Name}}
 		extcmds = append(extcmds, ec)
 	}
diff --git a/lfshttp/ssh.go b/lfshttp/ssh.go
index fc8bf77ccf..1176a44349 100644
--- a/lfshttp/ssh.go
+++ b/lfshttp/ssh.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os/exec"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -83,7 +82,7 @@ func (c *sshAuthClient) Resolve(e Endpoint, method string) (sshAuthResponse, err
 	}
 
 	exe, args := sshGetLFSExeAndArgs(c.os, c.git, e, method)
-	cmd := exec.Command(exe, args...)
+	cmd := subprocess.ExecCommand(exe, args...)
 
 	// Save stdout and stderr in separate buffers
 	var outbuf, errbuf bytes.Buffer
diff --git a/t/t-path.sh b/t/t-path.sh
index 3a5a5e0fb7..27fa21bd01 100755
--- a/t/t-path.sh
+++ b/t/t-path.sh
@@ -21,3 +21,41 @@ begin_test "does not look in current directory for git"
   ! grep -q 'exploit' output.log
 )
 end_test
+
+begin_test "does not look in current directory for git with credential helper"
+(
+  set -e
+
+  reponame="$(basename "$0" ".sh")-credentials"
+  setup_remote_repo "$reponame"
+
+  clone_repo "$reponame" credentials-1
+  export PATH="$(echo "$PATH" | sed -e "s/:.:/:/g" -e "s/::/:/g")"
+
+  printf "#!/bin/sh\necho exploit >&2\ntouch exploit\n" > git
+  chmod +x git || true
+  printf "echo exploit 1>&2\r\necho >exploit" > git.bat
+
+  git lfs track "*.dat"
+  printf abc > z.dat
+  git add z.dat
+  git add .gitattributes
+  git add git git.bat
+  git commit -m "Add files"
+
+  git push origin HEAD
+  cd ..
+
+  unset GIT_ASKPASS SSH_ASKPASS
+
+  # This needs to succeed.  If it fails, that could be because our malicious
+  # "git" is broken but got invoked anyway.
+  GIT_LFS_SKIP_SMUDGE=1 clone_repo "$reponame" credentials-2
+
+  git lfs pull | tee output.log
+
+  ! grep -q 'exploit' output.log
+  [ ! -f ../exploit ]
+  [ ! -f exploit ]
+)
+end_test