Merge pull request #2152 from AUTHENSOR/fix/extension-executor-shell-injection

cmd/generate_changelog/incoming/2152.txt

@@ -0,0 +1,3 @@
+### PR [#2152](https://github.com/danielmiessler/Fabric/pull/2152) by [AUTHENSOR](https://github.com/AUTHENSOR): fix: shell-escape extension values to prevent command injection
+
+- **Fix:** Shell-escape extension values to prevent command injection in the extension executor, which previously ran commands via `sh -c` with unescaped, user-controlled values interpolated into the command string. All user-controlled values are now wrapped in single quotes with embedded-single-quote escaping prior to interpolation, ensuring the shell treats them as literal arguments. A regression test (`ShellInjectionBlocked`) has been added to verify that malicious input (e.g., `hello; touch /marker`) does not execute unintended shell commands.

internal/plugins/template/extension_executor.go

@@ -78,20 +78,32 @@ func (e *ExtensionExecutor) formatCommand(ext *ExtensionDefinition, operation st
 		return "", fmt.Errorf("%s", fmt.Sprintf(i18n.T("extension_operation_not_found"), operation, ext.Name))
 	}
 
+	// Shell-escape all user-controlled values to prevent command injection.
+	// The command string is ultimately passed to "sh -c", so any shell
+	// metacharacters (;, |, $(), backticks, etc.) in the value would be
+	// executed. Wrapping each value in single quotes and escaping embedded
+	// single quotes ensures the value is treated as a literal argument.
 	vars := make(map[string]string)
 	vars["executable"] = ext.Executable
 	vars["operation"] = operation
-	vars["value"] = value
+	vars["value"] = shellEscape(value)
 
 	// Split on pipe for numbered variables
 	values := strings.Split(value, "|")
 	for i, val := range values {
-		vars[fmt.Sprintf("%d", i+1)] = val
+		vars[fmt.Sprintf("%d", i+1)] = shellEscape(val)
 	}
 
 	return ApplyTemplate(opConfig.CmdTemplate, vars, "")
 }
 
+// shellEscape wraps a string in single quotes for safe use in a shell command,
+// escaping any embedded single quotes. This prevents command injection when
+// untrusted input is passed as an argument to "sh -c".
+func shellEscape(s string) string {
+	return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'"
+}
+
 // executeStdout runs the command and captures its stdout
 func (e *ExtensionExecutor) executeStdout(cmd *exec.Cmd, ext *ExtensionDefinition) (string, error) {
 	var stdout bytes.Buffer

internal/plugins/template/extension_executor_test.go

@@ -21,6 +21,9 @@ case "$1" in
     "stdout")
         echo "Hello, $2!"
         ;;
+    "echo")
+        echo "$2"
+        ;;
     "file")
         echo "Hello, $2!" > "$3"
         echo "$3"  # Print the filename for path_from_stdout
@@ -39,6 +42,54 @@ esac`
 	registry := NewExtensionRegistry(tmpDir)
 	executor := NewExtensionExecutor(registry)
 
+	// Test that shell metacharacters in user input are neutralized.
+	// Before the fix, value flowed unescaped into "sh -c", so input
+	// like "; touch /tmp/pwned" would execute arbitrary commands.
+	t.Run("ShellInjectionBlocked", func(t *testing.T) {
+		// Use a marker file to detect if injection succeeded.
+		markerFile := filepath.Join(tmpDir, "injection-marker")
+		_ = os.Remove(markerFile)
+
+		configPath := filepath.Join(tmpDir, "inject-test.yaml")
+		configContent := `name: inject-test
+executable: ` + testScript + `
+type: executable
+timeout: 5s
+operations:
+  echo:
+    cmd_template: "{{executable}} echo {{value}}"
+config:
+  output:
+    method: stdout`
+
+		if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil {
+			t.Fatalf("Failed to create config: %v", err)
+		}
+
+		if err := registry.Register(configPath); err != nil {
+			t.Fatalf("Failed to register extension: %v", err)
+		}
+
+		// Malicious input: attempt to run a separate command after echo.
+		maliciousValue := "hello; touch " + markerFile
+
+		output, err := executor.Execute("inject-test", "echo", maliciousValue)
+		if err != nil {
+			t.Fatalf("Execute failed: %v", err)
+		}
+
+		// The output should contain the full malicious string as a literal
+		// argument (proving the shell did NOT interpret the semicolon).
+		if !strings.Contains(output, "hello; touch") {
+			t.Errorf("Expected literal value in output, got: %q", output)
+		}
+
+		// The marker file must NOT exist (injection was blocked).
+		if _, err := os.Stat(markerFile); !os.IsNotExist(err) {
+			t.Error("SECURITY: command injection succeeded — marker file was created")
+		}
+	})
+
 	// Test stdout-based extension
 	t.Run("StdoutExecution", func(t *testing.T) {
 		configPath := filepath.Join(tmpDir, "stdout-extension.yaml")