Skip to content

Commit 3f6e91b

Browse files
authored
fix(interp): gate $(<file) shortcut on cat being in the allowlist (#192)
* fix(interp): block $(<file) to prevent command allowlist bypass The $(<file) POSIX shortcut read files directly from cmdSubst, bypassing AllowedCommands. Reject any < redirect that is not paired with a command so the only way to read a file is through an allowed builtin (e.g. $(cat file)). * fix(interp): restrict < to file-reading builtins The previous commit rejected bare < redirects but accepted any command paired with <. That is broader than needed: `echo < file` would open a file purely to hand its stdin to a command that ignores it. Narrow the rule to the 11 file-reading builtins (cat, cut, grep, head, sed, sort, strings, tail, tr, uniq, wc) so < only applies where reading is the command's actual purpose. Also reject dynamic command names since they cannot be validated statically. * fix(interp): gate $(<file) shortcut on cat being in the allowlist Revert the overly-broad validation that blocked $(<file) entirely and restricted < to a fixed set of commands. The report's remediation keeps the shortcut functional but checks AllowedCommands at runtime: $(<file) is treated as an implicit $(cat file), so the read only proceeds when cat is allowed. This matches bash semantics and keeps legitimate uses working while closing the original allowlist bypass. * test(interp): extend $(<file) pentest coverage Add three regression tests to the existing pentest suite: - SandboxEnforcedWhenCatAllowed: the gate and AllowedPaths are independent defences. Allowing cat does not let a caller read files outside the sandbox. - SymlinkEscapeBlocked: a symlink inside AllowedPaths pointing outside must be refused by os.Root, not followed. - VariableExpandedPath: the shortcut runs word expansion on its path, both for legitimate use (F=data.txt; $(<$F)) and to guarantee the gate cannot be dodged by hiding the path behind a variable. * test(scenarios): assert exact stderr in cat_shortcut_bypass Per AGENTS.md, scenarios should prefer expect.stderr over stderr_contains when the output is deterministic. The blocked $(<file) gate emits a single fixed diagnostic and no variable sandbox warnings, so the contains-only form would miss extra stderr noise from a future regression. * test(scenarios): inverse of cat_shortcut_bypass — succeed when rshell:cat is allowed Pairs with blocked_redirects/cat_shortcut_bypass.yaml to prove the gate decision hinges purely on cat's membership in allowed_commands and has no other side effects on stdout/stderr.
1 parent 22c7c68 commit 3f6e91b

7 files changed

Lines changed: 356 additions & 2 deletions

File tree

SHELL_FEATURES.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ Blocked features are rejected before execution with exit code 2.
4242
- ✅ Expansion: `$VAR`, `${VAR}`
4343
-`$?` — last exit code (the only supported special variable)
4444
- ✅ Inline assignment: `VAR=value command` (scoped to that command)
45-
- ✅ Command substitution: `$(cmd)`, `` `cmd` `` — captures stdout; trailing newlines stripped; `$(<file)` shortcut reads file directly; output capped at 1 MiB
45+
- ✅ Command substitution: `$(cmd)`, `` `cmd` `` — captures stdout; trailing newlines stripped; `$(<file)` shortcut reads file directly (gated on `cat` being in the command allowlist); output capped at 1 MiB
4646
- ❌ Arithmetic expansion: `$(( expr ))`
4747
- ❌ Array assignment: `arr=(a b c)`, `arr[0]=x`
4848
- ❌ Append assignment: `VAR+=value`

interp/runner_expand.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,26 @@ const MaxGlobReadDirCalls = 10_000
6464

6565
// cmdSubst handles command substitution ($(...) and `...`).
6666
// It runs the commands in a subshell and writes their stdout to w.
67+
//
68+
// Special case: the POSIX `$(<file)` shortcut reads file contents
69+
// directly without spawning a subshell. Because that path performs a
70+
// file read without invoking any command, it would bypass the
71+
// AllowedCommands allowlist if left unchecked. We therefore gate the
72+
// shortcut on `cat` being an allowed command — `$(<file)` is treated as
73+
// an implicit `$(cat file)` for allowlist purposes.
6774
func (r *Runner) cmdSubst(w io.Writer, cs *syntax.CmdSubst) error {
6875
if len(cs.Stmts) == 0 {
6976
return nil
7077
}
7178

7279
// $(<file) shortcut: read file contents directly without a subshell.
7380
if word := catShortcutArg(cs.Stmts[0]); word != nil && len(cs.Stmts) == 1 {
81+
if !r.allowAllCommands && !r.allowedCommands["cat"] {
82+
r.errf("$(<file): file read not permitted (cat not in allowed commands)\n")
83+
r.lastExpandExit = exitStatus{code: 1}
84+
r.lastExit = r.lastExpandExit
85+
return nil
86+
}
7487
path := r.literal(word)
7588
f, err := r.open(r.ectx, path, os.O_RDONLY, 0, true)
7689
if err != nil {

interp/tests/cmdsubst_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,41 @@ func TestCmdSubstCatShortcutMissingFile(t *testing.T) {
182182
assert.Equal(t, "1\n", stdout, "$? should be 1 from the failed substitution")
183183
}
184184

185+
// TestCmdSubstCatShortcutCommandAllowlistBypass is the regression test
186+
// for the original vulnerability: $(<file) must be gated on `cat` being
187+
// in AllowedCommands. With the file in AllowedPaths but cat absent from
188+
// the allowlist, the shortcut must refuse to read.
189+
func TestCmdSubstCatShortcutCommandAllowlistBypass(t *testing.T) {
190+
dir := t.TempDir()
191+
require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("top secret"), 0644))
192+
stdout, stderr, code := cmdSubstRunWithOpts(t,
193+
`x=$(<secret.txt); echo "[$x]"`,
194+
dir,
195+
interp.AllowedPaths([]string{dir}),
196+
interp.AllowedCommands([]string{"rshell:echo"}),
197+
)
198+
assert.Equal(t, 0, code, "script completes; only the substitution fails")
199+
assert.Equal(t, "[]\n", stdout, "must not leak the file contents")
200+
assert.Contains(t, stderr, "file read not permitted")
201+
assert.Contains(t, stderr, "cat not in allowed commands")
202+
}
203+
204+
// TestCmdSubstCatShortcutAllowedWhenCatAllowed verifies the shortcut
205+
// works when cat is explicitly in AllowedCommands (not just
206+
// AllowAllCommands).
207+
func TestCmdSubstCatShortcutAllowedWhenCatAllowed(t *testing.T) {
208+
dir := t.TempDir()
209+
require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("ok"), 0644))
210+
stdout, _, code := cmdSubstRunWithOpts(t,
211+
`x=$(<data.txt); echo "$x"`,
212+
dir,
213+
interp.AllowedPaths([]string{dir}),
214+
interp.AllowedCommands([]string{"rshell:cat", "rshell:echo"}),
215+
)
216+
assert.Equal(t, 0, code)
217+
assert.Equal(t, "ok\n", stdout)
218+
}
219+
185220
// --- For loop integration ---
186221

187222
func TestCmdSubstInForLoop(t *testing.T) {
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2026-present Datadog, Inc.
5+
6+
package tests_test
7+
8+
import (
9+
"bytes"
10+
"context"
11+
"errors"
12+
"os"
13+
"path/filepath"
14+
"runtime"
15+
"strings"
16+
"testing"
17+
"time"
18+
19+
"github.com/stretchr/testify/assert"
20+
"github.com/stretchr/testify/require"
21+
"mvdan.cc/sh/v3/syntax"
22+
23+
"github.com/DataDog/rshell/interp"
24+
)
25+
26+
// Pentest suite for the `$(<file)` command-allowlist bypass.
27+
//
28+
// Historical vulnerability: the `$(<file)` POSIX shortcut in command
29+
// substitution was resolved by reading the file directly from the
30+
// interpreter, short-circuiting the normal command dispatch. That path
31+
// never consulted AllowedCommands, so a caller who had `rshell:echo`
32+
// whitelisted but not `rshell:cat` could still read any file inside
33+
// AllowedPaths via `x=$(<file); echo "$x"`.
34+
//
35+
// The mitigation keeps the shortcut functional but gates it on `cat`
36+
// being in the allowlist — $(<file) is treated as an implicit
37+
// $(cat file) for allowlist purposes. These tests verify both the gate
38+
// (reads refused when cat is missing) and the happy path (reads work
39+
// when cat is allowed).
40+
41+
// runBypass runs script with AllowedPaths=[dir] and the given allowed
42+
// commands. It returns stdout, stderr, and exit code.
43+
func runBypass(t *testing.T, script, dir string, allowedCmds []string) (string, string, int) {
44+
t.Helper()
45+
parser := syntax.NewParser()
46+
prog, err := parser.Parse(strings.NewReader(script), "")
47+
if err != nil {
48+
return "", err.Error(), 2
49+
}
50+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
51+
defer cancel()
52+
53+
var outBuf, errBuf bytes.Buffer
54+
opts := []interp.RunnerOption{
55+
interp.StdIO(nil, &outBuf, &errBuf),
56+
interp.AllowedPaths([]string{dir}),
57+
interp.AllowedCommands(allowedCmds),
58+
}
59+
runner, err := interp.New(opts...)
60+
require.NoError(t, err)
61+
defer runner.Close()
62+
runner.Dir = dir
63+
64+
err = runner.Run(ctx, prog)
65+
exitCode := 0
66+
if err != nil {
67+
var es interp.ExitStatus
68+
if errors.As(err, &es) {
69+
exitCode = int(es)
70+
} else if ctx.Err() == nil {
71+
t.Fatalf("unexpected error: %v", err)
72+
}
73+
}
74+
return outBuf.String(), errBuf.String(), exitCode
75+
}
76+
77+
// TestPentestCatShortcut_BlockedWhenCatNotAllowed is the exact exploit
78+
// reported: cat is NOT in the allowlist, but a caller tries to read a
79+
// file via $(<file). The gate must refuse without leaking content.
80+
func TestPentestCatShortcut_BlockedWhenCatNotAllowed(t *testing.T) {
81+
dir := t.TempDir()
82+
require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("TOP SECRET"), 0644))
83+
84+
stdout, stderr, code := runBypass(t,
85+
`x=$(<secret.txt); echo "[$x]"`,
86+
dir,
87+
[]string{"rshell:echo"},
88+
)
89+
// Script completes (echo runs); only the substitution fails with
90+
// $?=1 and a diagnostic message. stdout shows the empty
91+
// substitution, not the file contents.
92+
assert.Equal(t, 0, code)
93+
assert.Equal(t, "[]\n", stdout, "must not leak file contents")
94+
assert.Contains(t, stderr, "file read not permitted")
95+
assert.Contains(t, stderr, "cat not in allowed commands")
96+
assert.NotContains(t, stdout, "TOP SECRET")
97+
}
98+
99+
// TestPentestCatShortcut_AllowedWhenCatAllowed verifies the happy path:
100+
// with cat in the allowlist the shortcut reads the file as expected.
101+
func TestPentestCatShortcut_AllowedWhenCatAllowed(t *testing.T) {
102+
dir := t.TempDir()
103+
require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("content"), 0644))
104+
105+
stdout, stderr, code := runBypass(t,
106+
`x=$(<data.txt); echo "[$x]"`,
107+
dir,
108+
[]string{"rshell:cat", "rshell:echo"},
109+
)
110+
assert.Equal(t, 0, code, "should succeed when cat is allowed; stderr: %s", stderr)
111+
assert.Equal(t, "[content]\n", stdout)
112+
}
113+
114+
// TestPentestCatShortcut_ExitStatusPropagates verifies that the
115+
// blocked-shortcut sets $?=1 so scripts can detect the refusal with
116+
// standard control flow.
117+
func TestPentestCatShortcut_ExitStatusPropagates(t *testing.T) {
118+
dir := t.TempDir()
119+
require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("nope"), 0644))
120+
121+
stdout, _, code := runBypass(t,
122+
`x=$(<secret.txt); echo "$?"`,
123+
dir,
124+
[]string{"rshell:echo"},
125+
)
126+
assert.Equal(t, 0, code)
127+
assert.Equal(t, "1\n", stdout, "$? must be 1 after the blocked shortcut")
128+
}
129+
130+
// TestPentestCatShortcut_NoFileAccessWhenBlocked verifies that when the
131+
// gate refuses, no file is opened — a missing file yields the
132+
// allowlist-denial message, not a "no such file" error.
133+
func TestPentestCatShortcut_NoFileAccessWhenBlocked(t *testing.T) {
134+
dir := t.TempDir()
135+
_, stderr, code := runBypass(t,
136+
`x=$(<nonexistent-path.txt); echo "$x"`,
137+
dir,
138+
[]string{"rshell:echo"},
139+
)
140+
assert.Equal(t, 0, code)
141+
assert.Contains(t, stderr, "cat not in allowed commands")
142+
assert.NotContains(t, strings.ToLower(stderr), "no such file",
143+
"gate should short-circuit before any filesystem access")
144+
}
145+
146+
// TestPentestCatShortcut_SandboxEnforcedWhenCatAllowed verifies that
147+
// permitting `cat` in AllowedCommands does not weaken AllowedPaths.
148+
// Even with the gate satisfied, a path outside the sandbox must be
149+
// refused by the path layer — the two defences are independent.
150+
func TestPentestCatShortcut_SandboxEnforcedWhenCatAllowed(t *testing.T) {
151+
dir := t.TempDir()
152+
// Put a file somewhere outside `dir` so we have a concrete target
153+
// to attempt reading.
154+
outside := t.TempDir()
155+
outsidePath := filepath.Join(outside, "secret.txt")
156+
require.NoError(t, os.WriteFile(outsidePath, []byte("CROSS-BOUNDARY"), 0644))
157+
158+
// `echo "$?[$x]"` is evaluated before echo runs, so $? still
159+
// carries the substitution's exit code (1 on failure) and $x is
160+
// empty if the read was refused. The trailing echo prevents the
161+
// script's top-level exit code from being overwritten.
162+
script := `x=$(<` + outsidePath + `); echo "$?[$x]"`
163+
stdout, stderr, code := runBypass(t, script, dir,
164+
[]string{"rshell:cat", "rshell:echo"})
165+
166+
assert.Equal(t, 0, code)
167+
assert.Equal(t, "1[]\n", stdout,
168+
"substitution should fail ($?=1) and bind empty content")
169+
assert.NotContains(t, stdout, "CROSS-BOUNDARY", "file contents must not leak")
170+
assert.NotContains(t, stderr, "cat not in allowed commands",
171+
"failure should be path-level, not gate-level")
172+
assert.NotEmpty(t, stderr, "sandbox layer should have logged an error")
173+
}
174+
175+
// TestPentestCatShortcut_SymlinkEscapeBlocked verifies that a symlink
176+
// planted inside AllowedPaths pointing outside is refused by the
177+
// sandbox. The $(<file) shortcut uses the same r.open path as builtins,
178+
// so this is really a sanity check that symlinks do not bypass os.Root.
179+
func TestPentestCatShortcut_SymlinkEscapeBlocked(t *testing.T) {
180+
if runtime.GOOS == "windows" {
181+
t.Skip("symlink creation requires elevation on Windows")
182+
}
183+
dir := t.TempDir()
184+
outside := t.TempDir()
185+
outsidePath := filepath.Join(outside, "secret.txt")
186+
require.NoError(t, os.WriteFile(outsidePath, []byte("SYMLINK TARGET"), 0644))
187+
188+
// Plant a symlink inside the sandbox that points outside it.
189+
linkPath := filepath.Join(dir, "escape.lnk")
190+
require.NoError(t, os.Symlink(outsidePath, linkPath))
191+
192+
stdout, stderr, code := runBypass(t,
193+
`x=$(<escape.lnk); echo "$?[$x]"`,
194+
dir,
195+
[]string{"rshell:cat", "rshell:echo"})
196+
197+
assert.Equal(t, 0, code)
198+
assert.Equal(t, "1[]\n", stdout,
199+
"symlink escape should fail ($?=1) and bind empty content")
200+
assert.NotContains(t, stdout, "SYMLINK TARGET", "contents behind symlink must not leak")
201+
assert.NotContains(t, stderr, "cat not in allowed commands",
202+
"refusal should be sandbox-level, not gate-level")
203+
assert.NotEmpty(t, stderr, "sandbox layer should have logged an error")
204+
}
205+
206+
// TestPentestCatShortcut_VariableExpandedPath verifies that the
207+
// shortcut path goes through normal word expansion — an attacker
208+
// cannot dodge the allowlist check by hiding the path behind a
209+
// variable, and a legitimate user can use variables to build paths.
210+
func TestPentestCatShortcut_VariableExpandedPath(t *testing.T) {
211+
dir := t.TempDir()
212+
require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("expanded"), 0644))
213+
214+
// Happy path: expanded path works when cat is allowed.
215+
t.Run("allowed", func(t *testing.T) {
216+
stdout, stderr, code := runBypass(t,
217+
`F=data.txt; x=$(<$F); echo "[$x]"`,
218+
dir,
219+
[]string{"rshell:cat", "rshell:echo"})
220+
assert.Equal(t, 0, code, "should succeed; stderr: %s", stderr)
221+
assert.Equal(t, "[expanded]\n", stdout)
222+
})
223+
224+
// The gate fires regardless of whether the path is literal or
225+
// variable-expanded — variables cannot be used as an evasion.
226+
t.Run("blocked", func(t *testing.T) {
227+
stdout, stderr, code := runBypass(t,
228+
`F=data.txt; x=$(<$F); echo "[$x]"`,
229+
dir,
230+
[]string{"rshell:echo"})
231+
assert.Equal(t, 0, code)
232+
assert.Equal(t, "[]\n", stdout, "must not leak content via variable-expanded path")
233+
assert.Contains(t, stderr, "cat not in allowed commands")
234+
})
235+
}
236+
237+
// TestPentestCatShortcut_VariousContexts exercises the shortcut in
238+
// several expansion contexts (nested echo arg, if condition, for
239+
// iterator). In every context it must succeed when cat is allowed and
240+
// refuse when cat is not.
241+
func TestPentestCatShortcut_VariousContexts(t *testing.T) {
242+
dir := t.TempDir()
243+
require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("alpha"), 0644))
244+
245+
contexts := []struct {
246+
name string
247+
script string
248+
}{
249+
{"assignment", `x=$(<data.txt); echo "$x"`},
250+
{"echo arg", `echo "[$(<data.txt)]"`},
251+
{"backtick", "echo \"[`<data.txt`]\""},
252+
{"for iterator", `for x in $(<data.txt); do echo "$x"; done`},
253+
}
254+
255+
for _, tc := range contexts {
256+
t.Run(tc.name+"/allowed", func(t *testing.T) {
257+
stdout, stderr, code := runBypass(t, tc.script, dir,
258+
[]string{"rshell:cat", "rshell:echo"})
259+
assert.Equal(t, 0, code, "should succeed; stderr: %s", stderr)
260+
assert.Contains(t, stdout, "alpha")
261+
})
262+
t.Run(tc.name+"/blocked", func(t *testing.T) {
263+
stdout, stderr, code := runBypass(t, tc.script, dir,
264+
[]string{"rshell:echo"})
265+
assert.Equal(t, 0, code)
266+
assert.NotContains(t, stdout, "alpha",
267+
"blocked shortcut must not leak file contents")
268+
assert.Contains(t, stderr, "cat not in allowed commands")
269+
})
270+
}
271+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
description: $(<file) must not bypass the command allowlist — with the file in allowed_paths but cat NOT in allowed_commands, the shortcut is blocked at runtime.
2+
skip_assert_against_bash: true
3+
setup:
4+
files:
5+
- path: secret.txt
6+
content: "top secret"
7+
input:
8+
allowed_paths: ["$DIR"]
9+
allowed_commands: ["rshell:echo"]
10+
script: |+
11+
x=$(<secret.txt)
12+
echo "$x"
13+
expect:
14+
stdout: |+
15+
16+
stderr: |+
17+
$(<file): file read not permitted (cat not in allowed commands)
18+
exit_code: 0

tests/scenarios/shell/command_substitution/cat_shortcut.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
description: $(<file) shortcut reads file contents directly.
1+
description: $(<file) shortcut reads file contents directly when cat is allowed.
22
setup:
33
files:
44
- path: data.txt
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
description: $(<file) shortcut works when rshell:cat is explicitly in allowed_commands (inverse of blocked_redirects/cat_shortcut_bypass).
2+
skip_assert_against_bash: true
3+
setup:
4+
files:
5+
- path: data.txt
6+
content: "hello from file"
7+
input:
8+
allowed_paths: ["$DIR"]
9+
allowed_commands: ["rshell:cat", "rshell:echo"]
10+
script: |+
11+
x=$(<data.txt)
12+
echo "[$x]"
13+
expect:
14+
stdout: |+
15+
[hello from file]
16+
stderr: |+
17+
exit_code: 0

0 commit comments

Comments
 (0)