[Detail Bug] CLI: `amber run`/`amber eval` allow shell command injection via unescaped script arguments

[detail-app]

Summary

• Context: The AmberCompiler::execute method is responsible for running the compiled Bash code when a user executes an Amber script using amber run or amber eval.
• Bug: The method interpolates user-provided arguments into the Bash command string without proper escaping for shell-special characters (like $, `, and \).
• Actual vs. expected: Arguments are injected into a set -- ... command using double quotes, but only double-quote characters are escaped. Expected behavior is that arguments are passed as literal values to the script without being interpreted by the shell.
• Impact: This allows for arbitrary command injection and environment variable expansion on the host system whenever an Amber script is run with untrusted input.

Code with bug

pub fn execute(mut code: String, args: Vec<String>) -> Result<ExitStatus, std::io::Error> {
    if let Some(mut command) = Self::find_bash() {
        if !args.is_empty() {
            let args = args
                .into_iter()
                .map(|arg| arg.replace("\"", "\\\"")) // <-- BUG 🔴 [Only escapes quotes, not $, `, or \]
                .map(|arg| format!("\"{arg}\""))
                .collect::<Vec<String>>();
            code = format!("set -- {}\n{}", args.join(" "), code); // <-- BUG 🔴 [Direct interpolation into shell command]
        }
        command.arg("-c").arg(code).spawn()?.wait()
    } else {
        // ...
    }
}

Evidence

1. Command Injection

Running an Amber script with a command substitution argument triggers the execution of that command.

# Create a dummy Amber script
echo 'main(args) { echo args[1] }' > test.ab
# Run it with a malicious argument
./target/debug/amber run test.ab '$(touch exploit.txt)'
# Check if the command was executed
ls exploit.txt

Result: exploit.txt is created, confirming arbitrary command execution.

2. Variable Expansion

Running an Amber script with an environment variable reference expands it on the host.

./target/debug/amber run test.ab '$HOME'

Result: Prints /root (or the actual home directory) instead of the literal string $HOME.

3. Bash Syntax Error via Backslash

A trailing backslash in an argument escapes the closing double quote added by the compiler, leading to a broken Bash command.

./target/debug/amber run test.ab '\'

Result: bash: -c: line 6: unexpected EOF while looking for matching "', because the generated command becomes set -- "\"\n....

Why has this bug gone undetected?

This bug has likely gone undetected because amber run is primarily used during development where the user is providing their own arguments. Developers are unlikely to pass malicious or complex strings ending in backslashes to their own scripts during testing. Furthermore, scripts compiled to standalone Bash files using amber build are not affected, as the generated Bash code correctly uses "$@" to handle positional parameters.

Recommended fix

The correct way to pass arguments to a Bash script executed via bash -c is to pass them as additional positional parameters to the bash command itself, rather than interpolating them into the script string.

pub fn execute(code: String, args: Vec<String>) -> Result<ExitStatus, std::io::Error> {
    if let Some(mut command) = Self::find_bash() {
        command.arg("-c").arg(code).arg("amber").args(args).spawn()?.wait() // <-- FIX 🟢
    } else {
        // ...
    }
}

This approach ensures that $1, $2, etc. are correctly populated in the script while being treated as literal values by the shell. Note that the third argument to bash -c (here "amber") sets the value of $0 for the script.

Related bugs

• Sudo Prefix Concatenation: The sudo modifier fails on non-root systems because it generates ${__sudo}command (e.g., sudols) instead of ${__sudo} command.
• Sudo Variable Pollution: The __sudo variable is not cleared before use, allowing it to be hijacked by environment variables.

History

This bug was introduced in commit e5467d2 (@hdwalters, 2024-11-21, PR #600). The change aimed to support passing CLI arguments to Amber scripts by interpolating them into a set -- command, but it used insufficient escaping that only handled double quotes and left the system vulnerable to command injection and variable expansion.