-
Notifications
You must be signed in to change notification settings - Fork 1
fix(interp): gate $(<file) shortcut on cat being in the allowlist #192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f515613
fix(interp): block $(<file) to prevent command allowlist bypass
matt-dz 397aaeb
fix(interp): restrict < to file-reading builtins
matt-dz ebc4a66
fix(interp): gate $(<file) shortcut on cat being in the allowlist
matt-dz 4f45c64
test(interp): extend $(<file) pentest coverage
matt-dz a695350
test(scenarios): assert exact stderr in cat_shortcut_bypass
matt-dz 084e0cc
test(scenarios): inverse of cat_shortcut_bypass — succeed when rshell…
matt-dz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,271 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2026-present Datadog, Inc. | ||
|
|
||
| package tests_test | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "errors" | ||
| "os" | ||
| "path/filepath" | ||
| "runtime" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "mvdan.cc/sh/v3/syntax" | ||
|
|
||
| "github.com/DataDog/rshell/interp" | ||
| ) | ||
|
|
||
| // Pentest suite for the `$(<file)` command-allowlist bypass. | ||
| // | ||
| // Historical vulnerability: the `$(<file)` POSIX shortcut in command | ||
| // substitution was resolved by reading the file directly from the | ||
| // interpreter, short-circuiting the normal command dispatch. That path | ||
| // never consulted AllowedCommands, so a caller who had `rshell:echo` | ||
| // whitelisted but not `rshell:cat` could still read any file inside | ||
| // AllowedPaths via `x=$(<file); echo "$x"`. | ||
| // | ||
| // The mitigation keeps the shortcut functional but gates it on `cat` | ||
| // being in the allowlist — $(<file) is treated as an implicit | ||
| // $(cat file) for allowlist purposes. These tests verify both the gate | ||
| // (reads refused when cat is missing) and the happy path (reads work | ||
| // when cat is allowed). | ||
|
|
||
| // runBypass runs script with AllowedPaths=[dir] and the given allowed | ||
| // commands. It returns stdout, stderr, and exit code. | ||
| func runBypass(t *testing.T, script, dir string, allowedCmds []string) (string, string, int) { | ||
| t.Helper() | ||
| parser := syntax.NewParser() | ||
| prog, err := parser.Parse(strings.NewReader(script), "") | ||
| if err != nil { | ||
| return "", err.Error(), 2 | ||
| } | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
|
|
||
| var outBuf, errBuf bytes.Buffer | ||
| opts := []interp.RunnerOption{ | ||
| interp.StdIO(nil, &outBuf, &errBuf), | ||
| interp.AllowedPaths([]string{dir}), | ||
| interp.AllowedCommands(allowedCmds), | ||
| } | ||
| runner, err := interp.New(opts...) | ||
| require.NoError(t, err) | ||
| defer runner.Close() | ||
| runner.Dir = dir | ||
|
|
||
| err = runner.Run(ctx, prog) | ||
| exitCode := 0 | ||
| if err != nil { | ||
| var es interp.ExitStatus | ||
| if errors.As(err, &es) { | ||
| exitCode = int(es) | ||
| } else if ctx.Err() == nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| } | ||
| return outBuf.String(), errBuf.String(), exitCode | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_BlockedWhenCatNotAllowed is the exact exploit | ||
| // reported: cat is NOT in the allowlist, but a caller tries to read a | ||
| // file via $(<file). The gate must refuse without leaking content. | ||
| func TestPentestCatShortcut_BlockedWhenCatNotAllowed(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("TOP SECRET"), 0644)) | ||
|
|
||
| stdout, stderr, code := runBypass(t, | ||
| `x=$(<secret.txt); echo "[$x]"`, | ||
| dir, | ||
| []string{"rshell:echo"}, | ||
| ) | ||
| // Script completes (echo runs); only the substitution fails with | ||
| // $?=1 and a diagnostic message. stdout shows the empty | ||
| // substitution, not the file contents. | ||
| assert.Equal(t, 0, code) | ||
| assert.Equal(t, "[]\n", stdout, "must not leak file contents") | ||
| assert.Contains(t, stderr, "file read not permitted") | ||
| assert.Contains(t, stderr, "cat not in allowed commands") | ||
| assert.NotContains(t, stdout, "TOP SECRET") | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_AllowedWhenCatAllowed verifies the happy path: | ||
| // with cat in the allowlist the shortcut reads the file as expected. | ||
| func TestPentestCatShortcut_AllowedWhenCatAllowed(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("content"), 0644)) | ||
|
|
||
| stdout, stderr, code := runBypass(t, | ||
| `x=$(<data.txt); echo "[$x]"`, | ||
| dir, | ||
| []string{"rshell:cat", "rshell:echo"}, | ||
| ) | ||
| assert.Equal(t, 0, code, "should succeed when cat is allowed; stderr: %s", stderr) | ||
| assert.Equal(t, "[content]\n", stdout) | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_ExitStatusPropagates verifies that the | ||
| // blocked-shortcut sets $?=1 so scripts can detect the refusal with | ||
| // standard control flow. | ||
| func TestPentestCatShortcut_ExitStatusPropagates(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("nope"), 0644)) | ||
|
|
||
| stdout, _, code := runBypass(t, | ||
| `x=$(<secret.txt); echo "$?"`, | ||
| dir, | ||
| []string{"rshell:echo"}, | ||
| ) | ||
| assert.Equal(t, 0, code) | ||
| assert.Equal(t, "1\n", stdout, "$? must be 1 after the blocked shortcut") | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_NoFileAccessWhenBlocked verifies that when the | ||
| // gate refuses, no file is opened — a missing file yields the | ||
| // allowlist-denial message, not a "no such file" error. | ||
| func TestPentestCatShortcut_NoFileAccessWhenBlocked(t *testing.T) { | ||
| dir := t.TempDir() | ||
| _, stderr, code := runBypass(t, | ||
| `x=$(<nonexistent-path.txt); echo "$x"`, | ||
| dir, | ||
| []string{"rshell:echo"}, | ||
| ) | ||
| assert.Equal(t, 0, code) | ||
| assert.Contains(t, stderr, "cat not in allowed commands") | ||
| assert.NotContains(t, strings.ToLower(stderr), "no such file", | ||
| "gate should short-circuit before any filesystem access") | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_SandboxEnforcedWhenCatAllowed verifies that | ||
| // permitting `cat` in AllowedCommands does not weaken AllowedPaths. | ||
| // Even with the gate satisfied, a path outside the sandbox must be | ||
| // refused by the path layer — the two defences are independent. | ||
| func TestPentestCatShortcut_SandboxEnforcedWhenCatAllowed(t *testing.T) { | ||
| dir := t.TempDir() | ||
| // Put a file somewhere outside `dir` so we have a concrete target | ||
| // to attempt reading. | ||
| outside := t.TempDir() | ||
| outsidePath := filepath.Join(outside, "secret.txt") | ||
| require.NoError(t, os.WriteFile(outsidePath, []byte("CROSS-BOUNDARY"), 0644)) | ||
|
|
||
| // `echo "$?[$x]"` is evaluated before echo runs, so $? still | ||
| // carries the substitution's exit code (1 on failure) and $x is | ||
| // empty if the read was refused. The trailing echo prevents the | ||
| // script's top-level exit code from being overwritten. | ||
| script := `x=$(<` + outsidePath + `); echo "$?[$x]"` | ||
| stdout, stderr, code := runBypass(t, script, dir, | ||
| []string{"rshell:cat", "rshell:echo"}) | ||
|
|
||
| assert.Equal(t, 0, code) | ||
| assert.Equal(t, "1[]\n", stdout, | ||
| "substitution should fail ($?=1) and bind empty content") | ||
| assert.NotContains(t, stdout, "CROSS-BOUNDARY", "file contents must not leak") | ||
| assert.NotContains(t, stderr, "cat not in allowed commands", | ||
| "failure should be path-level, not gate-level") | ||
| assert.NotEmpty(t, stderr, "sandbox layer should have logged an error") | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_SymlinkEscapeBlocked verifies that a symlink | ||
| // planted inside AllowedPaths pointing outside is refused by the | ||
| // sandbox. The $(<file) shortcut uses the same r.open path as builtins, | ||
| // so this is really a sanity check that symlinks do not bypass os.Root. | ||
| func TestPentestCatShortcut_SymlinkEscapeBlocked(t *testing.T) { | ||
| if runtime.GOOS == "windows" { | ||
| t.Skip("symlink creation requires elevation on Windows") | ||
| } | ||
| dir := t.TempDir() | ||
| outside := t.TempDir() | ||
| outsidePath := filepath.Join(outside, "secret.txt") | ||
| require.NoError(t, os.WriteFile(outsidePath, []byte("SYMLINK TARGET"), 0644)) | ||
|
|
||
| // Plant a symlink inside the sandbox that points outside it. | ||
| linkPath := filepath.Join(dir, "escape.lnk") | ||
| require.NoError(t, os.Symlink(outsidePath, linkPath)) | ||
|
|
||
| stdout, stderr, code := runBypass(t, | ||
| `x=$(<escape.lnk); echo "$?[$x]"`, | ||
| dir, | ||
| []string{"rshell:cat", "rshell:echo"}) | ||
|
|
||
| assert.Equal(t, 0, code) | ||
| assert.Equal(t, "1[]\n", stdout, | ||
| "symlink escape should fail ($?=1) and bind empty content") | ||
| assert.NotContains(t, stdout, "SYMLINK TARGET", "contents behind symlink must not leak") | ||
| assert.NotContains(t, stderr, "cat not in allowed commands", | ||
| "refusal should be sandbox-level, not gate-level") | ||
| assert.NotEmpty(t, stderr, "sandbox layer should have logged an error") | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_VariableExpandedPath verifies that the | ||
| // shortcut path goes through normal word expansion — an attacker | ||
| // cannot dodge the allowlist check by hiding the path behind a | ||
| // variable, and a legitimate user can use variables to build paths. | ||
| func TestPentestCatShortcut_VariableExpandedPath(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("expanded"), 0644)) | ||
|
|
||
| // Happy path: expanded path works when cat is allowed. | ||
| t.Run("allowed", func(t *testing.T) { | ||
| stdout, stderr, code := runBypass(t, | ||
| `F=data.txt; x=$(<$F); echo "[$x]"`, | ||
| dir, | ||
| []string{"rshell:cat", "rshell:echo"}) | ||
| assert.Equal(t, 0, code, "should succeed; stderr: %s", stderr) | ||
| assert.Equal(t, "[expanded]\n", stdout) | ||
| }) | ||
|
|
||
| // The gate fires regardless of whether the path is literal or | ||
| // variable-expanded — variables cannot be used as an evasion. | ||
| t.Run("blocked", func(t *testing.T) { | ||
| stdout, stderr, code := runBypass(t, | ||
| `F=data.txt; x=$(<$F); echo "[$x]"`, | ||
| dir, | ||
| []string{"rshell:echo"}) | ||
| assert.Equal(t, 0, code) | ||
| assert.Equal(t, "[]\n", stdout, "must not leak content via variable-expanded path") | ||
| assert.Contains(t, stderr, "cat not in allowed commands") | ||
| }) | ||
| } | ||
|
|
||
| // TestPentestCatShortcut_VariousContexts exercises the shortcut in | ||
| // several expansion contexts (nested echo arg, if condition, for | ||
| // iterator). In every context it must succeed when cat is allowed and | ||
| // refuse when cat is not. | ||
| func TestPentestCatShortcut_VariousContexts(t *testing.T) { | ||
| dir := t.TempDir() | ||
| require.NoError(t, os.WriteFile(filepath.Join(dir, "data.txt"), []byte("alpha"), 0644)) | ||
|
|
||
| contexts := []struct { | ||
| name string | ||
| script string | ||
| }{ | ||
| {"assignment", `x=$(<data.txt); echo "$x"`}, | ||
| {"echo arg", `echo "[$(<data.txt)]"`}, | ||
| {"backtick", "echo \"[`<data.txt`]\""}, | ||
| {"for iterator", `for x in $(<data.txt); do echo "$x"; done`}, | ||
| } | ||
|
|
||
| for _, tc := range contexts { | ||
| t.Run(tc.name+"/allowed", func(t *testing.T) { | ||
| stdout, stderr, code := runBypass(t, tc.script, dir, | ||
| []string{"rshell:cat", "rshell:echo"}) | ||
| assert.Equal(t, 0, code, "should succeed; stderr: %s", stderr) | ||
| assert.Contains(t, stdout, "alpha") | ||
| }) | ||
| t.Run(tc.name+"/blocked", func(t *testing.T) { | ||
| stdout, stderr, code := runBypass(t, tc.script, dir, | ||
| []string{"rshell:echo"}) | ||
| assert.Equal(t, 0, code) | ||
| assert.NotContains(t, stdout, "alpha", | ||
| "blocked shortcut must not leak file contents") | ||
| assert.Contains(t, stderr, "cat not in allowed commands") | ||
| }) | ||
| } | ||
| } |
18 changes: 18 additions & 0 deletions
18
tests/scenarios/shell/blocked_redirects/cat_shortcut_bypass.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| 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. | ||
| skip_assert_against_bash: true | ||
| setup: | ||
| files: | ||
| - path: secret.txt | ||
| content: "top secret" | ||
| input: | ||
| allowed_paths: ["$DIR"] | ||
| allowed_commands: ["rshell:echo"] | ||
|
matt-dz marked this conversation as resolved.
|
||
| script: |+ | ||
| x=$(<secret.txt) | ||
| echo "$x" | ||
| expect: | ||
| stdout: |+ | ||
|
|
||
| stderr: |+ | ||
| $(<file): file read not permitted (cat not in allowed commands) | ||
| exit_code: 0 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
tests/scenarios/shell/command_substitution/cat_shortcut_allowed_explicit.yaml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| description: $(<file) shortcut works when rshell:cat is explicitly in allowed_commands (inverse of blocked_redirects/cat_shortcut_bypass). | ||
| skip_assert_against_bash: true | ||
| setup: | ||
| files: | ||
| - path: data.txt | ||
| content: "hello from file" | ||
| input: | ||
| allowed_paths: ["$DIR"] | ||
| allowed_commands: ["rshell:cat", "rshell:echo"] | ||
| script: |+ | ||
| x=$(<data.txt) | ||
| echo "[$x]" | ||
| expect: | ||
| stdout: |+ | ||
| [hello from file] | ||
| stderr: |+ | ||
| exit_code: 0 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.