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.
This commit is contained in:
brian m. carlson 2020-12-21 21:08:17 +00:00
parent 7b8779f3d4
commit 10c4ffc6b8
No known key found for this signature in database
GPG Key ID: 2D0C9BC12F82B3A1
5 changed files with 46 additions and 9 deletions

@ -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

@ -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
/*

@ -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)
}

@ -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

@ -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