execute: avoid interpolating args into bash -c

Author: RobertoLuzanilla

internal/execute/execute.go

@@ -36,17 +36,18 @@ func NewExpression(expression string, args []string) Command {
 
 // Run executes a Command with Bash and returns the error if there is one
 func (c Command) Run() error {
-	var command string
+	var bashArgs []string
+
 	if c.Expression != "" {
-		// Expressions need to be invoked inside a Bash function, so variables like
-		// $0 and $@ are available
-		command = fmt.Sprintf("fn() { %s; }; fn %s", c.Expression, formatArgString(c.Args))
+		script := fmt.Sprintf(`fn() { %s; }; fn "$@"`, c.Expression)
+		bashArgs = append([]string{"-c", script, "asdf"}, c.Args...)
+	} else if len(c.Args) > 0 {
+		bashArgs = append([]string{"-c", `exec "$0" "$@"`, c.Command}, c.Args...)
 	} else {
-		// Scripts can be invoked directly, with args provided
-		command = fmt.Sprintf("%s %s", c.Command, formatArgString(c.Args))
+		bashArgs = []string{"-c", c.Command}
 	}
 
-	cmd := exec.Command("bash", "-c", command)
+	cmd := exec.Command("bash", bashArgs...)
 
 	if len(c.Env) > 0 {
 		cmd.Env = MergeWithCurrentEnv(c.Env)
@@ -55,8 +56,6 @@ func (c Command) Run() error {
 	}
 
 	cmd.Stdin = c.Stdin
-
-	// Capture stdout and stderr
 	cmd.Stdout = c.Stdout
 	cmd.Stderr = c.Stderr
 
@@ -105,11 +104,3 @@ func SliceToMap(env []string) map[string]string {
 
 	return envMap
 }
-
-func formatArgString(args []string) string {
-	var newArgs []string
-	for _, str := range args {
-		newArgs = append(newArgs, fmt.Sprintf("\"%s\"", str))
-	}
-	return strings.Join(newArgs, " ")
-}

internal/execute/execute_test.go

@@ -2,6 +2,7 @@ package execute
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"strings"
@@ -18,6 +19,22 @@ func TestNew(t *testing.T) {
 	})
 }
 
+func TestRun_NoArgInjection(t *testing.T) {
+	tmp := t.TempDir()
+	injected := tmp + "/injected"
+
+	payload := fmt.Sprintf(`http://x"; : > %s #`, injected)
+
+	cmd := New("git", []string{"clone", payload, tmp + "/noop"})
+	cmd.Stdout = io.Discard
+	cmd.Stderr = io.Discard
+	_ = cmd.Run()
+
+	if _, err := os.Stat(injected); err == nil {
+		t.Fatalf("injection succeeded: %s was created", injected)
+	}
+}
+
 func TestNewExpression(t *testing.T) {
 	t.Run("Returns new command expression", func(t *testing.T) {
 		cmd := NewExpression("echo", []string{"test string"})