From e54ce2dbfff394ea6c57bb270994f09a4d8adfd4 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman <91019033+matt-dz@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:05:58 -0400 Subject: [PATCH 1/3] fix(interp): bound script nesting depth to prevent stack-overflow crash A sub-MaxScriptBytes but deeply nested script could drive the recursive descent parser, the recursive validateNode walk, or the recursive executor past the Go runtime's 1 GiB goroutine-stack limit, aborting the embedding process with an uncatchable "fatal error: stack overflow" before any command, path, mode, or timeout policy was consulted. The recover() guard in Runner.Run cannot intercept a runtime-fatal throw. Add MaxNestingDepth (5000) enforced two ways: - A quoting-aware lexical pre-scan in ParseScript bounds the nesting of bracketing/substitution constructs and the if/elif/while/until/for/select/ case compound keywords before the parser descends into them. A context stack keeps keyword-looking words inside quoted strings literal, while still counting keywords nested inside command substitutions. - An iterative AST-depth check at the top of validateNode (reached from both Run and Check) bounds pipeline and logical-list depth, which the parser builds iteratively but the later walks and execution recurse over. It reads a node's direct children via a depth-one syntax.Walk, so it never recurses and cannot itself overflow. Covers VULN-4562, VULN-1673, and VULN-4535. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- README.md | 2 + SHELL_FEATURES.md | 2 +- interp/api.go | 5 + interp/nesting.go | 337 +++++++++++++++++++++++++++++++ interp/nesting_vuln_hunt_test.go | 189 +++++++++++++++++ interp/validate.go | 6 + 6 files changed, 540 insertions(+), 1 deletion(-) create mode 100644 interp/nesting.go create mode 100644 interp/nesting_vuln_hunt_test.go diff --git a/README.md b/README.md index fb85c27e2..f1f49f03f 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,8 @@ Sed selects the longest regex match at the leftmost position, including for in-p For simple commands, rshell expands only enough words to identify the command, then applies command, elevation, and mode policy before expanding the remaining arguments, inline assignments, or redirects. A rejected command therefore cannot trigger command substitutions or redirect side effects through its unused arguments. Once a command is authorized, redirects retain normal shell semantics: they are established before execution and remain effective even if the command later exits nonzero. +Script text is bounded before parsing: at most 5 MiB of source, and a nesting depth (parentheses, substitutions, brace groups, pipelines, logical lists, and `if`/`while`/`for`/`case` compounds) of at most 5000. Both are enforced before and during AST validation so that a deeply nested but sub-5-MiB script cannot overflow the parser's or interpreter's recursion and crash the embedding process. + Shell expansion is bounded independently of the execution timeout: a command may receive at most 16,384 expanded arguments totaling 10 MiB, and one `Run` may produce at most 64 MiB of expanded argument, assignment, redirect, and heredoc text across all loops, subshells, and pipeline stages. Individual variable values remain capped at 1 MiB and heredocs at 10 MiB. Redirect targets undergo shell expansion and must resolve to exactly one field; ambiguous targets fail before the file is opened. Quoting preserves literal spaces and wildcard characters. diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index a72529c87..c22ad5bc3 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -22,7 +22,7 @@ the Go error return. | Surface | Static check | |---|---| -| Shell syntax | Same `ParseScript` size limit and restricted AST validation as execution | +| Shell syntax | Same `ParseScript` size and nesting-depth limits, and restricted AST validation, as execution | | Commands | Exact `AllowedCommands`, builtin registration, remediation-only metadata, selective elevation, and prohibition of elevation in pipelines | | Flags | Same builtin flag factories, normalization, and parsing as execution; handler-level semantic checks and embedded AWK/sed/jq programs are not validated | | Paths | Explicit redirects and modeled builtin operands; local checks use sandbox metadata for containment and write restrictions; remote checks use only lexical grant parsing, containment, and access precedence, leaving target metadata indeterminate | diff --git a/interp/api.go b/interp/api.go index dea61bd0d..e7b2a6e19 100644 --- a/interp/api.go +++ b/interp/api.go @@ -812,6 +812,11 @@ func ParseScript(script, name string) (*syntax.File, error) { return nil, fmt.Errorf("script size %d bytes exceeds maximum of %d bytes (%d MiB); split the script into smaller pieces", len(script), MaxScriptBytes, MaxScriptBytes/(1024*1024)) } + // Reject deeply nested scripts before the recursive-descent parser can + // overflow the goroutine stack on them. See [MaxNestingDepth]. + if err := checkScriptNestingDepth(script); err != nil { + return nil, err + } return syntax.NewParser().Parse(strings.NewReader(script), name) } diff --git a/interp/nesting.go b/interp/nesting.go new file mode 100644 index 000000000..1e82e7a76 --- /dev/null +++ b/interp/nesting.go @@ -0,0 +1,337 @@ +// 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 interp + +import ( + "fmt" + + "mvdan.cc/sh/v3/syntax" +) + +// MaxNestingDepth bounds how deeply a script may nest shell constructs +// (parentheses, command/parameter substitutions, brace groups, pipelines, +// logical lists, and compound commands such as if/while/for/case). +// +// The bound exists purely for safety, not for compatibility: the recursive +// descent parser in mvdan.cc/sh, rshell's own recursive AST passes +// ([validateNode] via [syntax.Walk]), and the recursive executor all consume +// Go stack frames per nesting level. A sufficiently deep script — still well +// under [MaxScriptBytes] — grows the goroutine stack past the Go runtime's +// 1 GiB limit, which aborts the process with an *uncatchable* "fatal error: +// stack overflow". That throw is not a panic, so the recover() guard in +// [Runner.Run] cannot intercept it, and it happens before any command, path, +// mode, or timeout policy is consulted. +// +// The pre-parse lexical guard ([checkScriptNestingDepth]) measures this in +// source bracket depth; the post-parse guard ([checkNestingDepth]) measures it +// in AST node depth, which is a small constant multiple of the source nesting +// (each nested construct expands to a few AST levels). 5000 is far below the +// depth at which any of those recursions overflows (empirically the parser +// alone tolerates tens of thousands of bracket levels, and the crash is at +// hundreds of thousands) yet comfortably above the nesting depth of any +// realistic script — including long pipelines — so legitimate input is never +// rejected while the crash is prevented. +const MaxNestingDepth = 5000 + +// errNestingTooDeep is the error reported when a script exceeds +// [MaxNestingDepth]. It is shared by the pre-parse lexical guard and the +// post-parse AST guard so callers see a consistent message. +func errNestingTooDeep() error { + return fmt.Errorf("script nesting exceeds maximum depth of %d; simplify deeply nested constructs", MaxNestingDepth) +} + +// Frame kinds tracked by the lexical pre-scan's context stack. +type nestKind uint8 + +const ( + kindParen nestKind = iota // ( ... ) — subshell, $( ), arithmetic grouping + kindBrace // { ... } — brace group, ${ } + kindBacktick // ` ... ` + kindIf // if ... (closed by fi) + kindElif // elif ... (part of an if ladder, closed by fi) + kindLoop // while/until/for/select ... (closed by done) + kindCase // case ... (closed by esac) + kindSingle // ' ... ' (lexical only, not a nesting level) + kindDouble // " ... " (lexical only, not a nesting level) +) + +// checkScriptNestingDepth is a cheap lexical pre-scan run by [ParseScript] +// before the script text is handed to the recursive-descent parser. Its sole +// job is to stop the parser itself from overflowing the stack on deeply nested +// constructs — subshells and substitutions "(" / "$(" / "${" / "$((", +// backquotes, brace groups "{", and the compound keyword commands if/elif, +// while/until/for/select, and case — each of which the parser descends into +// once per nesting level, before any AST (and therefore the post-parse +// [checkNestingDepth] guard) exists. +// +// It is a small quoting-aware state machine over a context stack: the top +// frame determines whether the scanner is in command context (where brackets +// and reserved words open nested constructs), inside single quotes (fully +// literal), or inside double quotes (where only "$(", "${" and backquotes open +// a nested command context — so ordinary words like for/do/if appearing in a +// quoted string are not miscounted). A command substitution opened inside +// double quotes pushes a command-context frame, so reserved words nested +// within it are still counted. +// +// Pipelines and &&/|| lists are deliberately not counted: the parser builds +// those iteratively (no per-stage stack growth), so they cannot overflow it; +// their depth is bounded after parsing by [checkNestingDepth] instead. +// +// The scan is a conservative upper bound on the parser's recursion depth — it +// may count a construct the parser would treat as literal — so it can only +// ever reject slightly early, never let a crashing script through. Because the +// threshold is far above any real nesting, this has no practical effect on +// legitimate scripts. +func checkScriptNestingDepth(script string) error { + stack := make([]nestKind, 0, 64) + depth := 0 // number of stacked frames that are genuine nesting levels + + top := func() (nestKind, bool) { + if len(stack) == 0 { + return 0, false + } + return stack[len(stack)-1], true + } + push := func(k nestKind) error { + stack = append(stack, k) + if k != kindSingle && k != kindDouble { + depth++ + if depth > MaxNestingDepth { + return errNestingTooDeep() + } + } + return nil + } + pop := func() { + k := stack[len(stack)-1] + stack = stack[:len(stack)-1] + if k != kindSingle && k != kindDouble { + depth-- + } + } + // mode reports the lexing context implied by the top frame. + inSingle := func() bool { k, ok := top(); return ok && k == kindSingle } + inDouble := func() bool { k, ok := top(); return ok && k == kindDouble } + + atWordStart := true + isDelim := func(c byte) bool { + switch c { + case ' ', '\t', '\n', '\r', ';', '&', '|', '(', ')', '<', '>', '`': + return true + } + return false + } + + for i := 0; i < len(script); i++ { + c := script[i] + + if inSingle() { + if c == '\'' { + pop() + } + continue + } + + if inDouble() { + switch c { + case '\\': + i++ // escape next byte + case '"': + pop() + case '`': + if err := push(kindBacktick); err != nil { + return err + } + case '$': + if i+1 < len(script) { + switch script[i+1] { + case '(': + if err := push(kindParen); err != nil { + return err + } + i++ + case '{': + if err := push(kindBrace); err != nil { + return err + } + i++ + } + } + } + continue + } + + // Command context (base frame or any command-context frame on top). + switch c { + case '\\': + i++ // escape next byte + atWordStart = false + case '\'': + if err := push(kindSingle); err != nil { + return err + } + atWordStart = false + case '"': + if err := push(kindDouble); err != nil { + return err + } + atWordStart = false + case '`': + if k, ok := top(); ok && k == kindBacktick { + pop() // close the backquote + } else if err := push(kindBacktick); err != nil { + return err + } + atWordStart = false + case '#': + if atWordStart { + for i < len(script) && script[i] != '\n' { + i++ + } + atWordStart = true + } else { + atWordStart = false + } + case '(': + if err := push(kindParen); err != nil { + return err + } + atWordStart = true + case '{': + if err := push(kindBrace); err != nil { + return err + } + atWordStart = true + case ')': + if k, ok := top(); ok && k == kindParen { + pop() + } + atWordStart = true + case '}': + if k, ok := top(); ok && k == kindBrace { + pop() + } + atWordStart = true + case ' ', '\t', '\n', '\r', ';', '&', '|': + atWordStart = true + default: + if atWordStart && isASCIILetter(c) { + // Read the maximal run of ASCII letters and decide whether it + // is a standalone reserved word. + j := i + 1 + for j < len(script) && isASCIILetter(script[j]) { + j++ + } + word := script[i:j] + following := byte(' ') + if j < len(script) { + following = script[j] + } + if j == len(script) || isDelim(following) { + if err := applyKeyword(word, &stack, push, pop); err != nil { + return err + } + } + i = j - 1 // advance past the letter run + } + atWordStart = false + } + } + return nil +} + +func isASCIILetter(c byte) bool { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') +} + +// applyKeyword updates the context stack for a standalone reserved word. Only +// the compound-command keywords that make the parser descend a level are +// significant; other words (then, do, in, else, and any non-keyword) are +// ignored. +func applyKeyword(word string, stack *[]nestKind, push func(nestKind) error, pop func()) error { + switch word { + case "if": + return push(kindIf) + case "elif": + return push(kindElif) + case "while", "until", "for", "select": + return push(kindLoop) + case "case": + return push(kindCase) + case "fi": + // Close an if ladder: pop any elif frames, then the matching if. + for len(*stack) > 0 && (*stack)[len(*stack)-1] == kindElif { + pop() + } + if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindIf { + pop() + } + case "done": + if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindLoop { + pop() + } + case "esac": + if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindCase { + pop() + } + } + return nil +} + +// checkNestingDepth bounds the structural depth of a parsed AST. It is called +// from [validateNode] — and therefore on every [Runner.Run] and [Runner.Check] +// path — before the recursive [syntax.Walk] validation pass and before any +// recursive execution, so that none of those recursions can exceed +// [MaxNestingDepth] stack frames. +// +// It is itself iterative: it never recurses deeper than one level (to read a +// node's direct children), so it cannot overflow the stack regardless of how +// deep the AST is. Constructs that the recursive-descent parser accepts +// without recursing per level — notably long pipelines (a|b|c…) and logical +// lists (a&&b&&c…), which build a left-nested BinaryCmd tree whose depth the +// parser does not pay for in stack but whose later walks and execution do — +// are bounded here even though the lexical pre-scan does not see them. +func checkNestingDepth(root syntax.Node) error { + if root == nil { + return nil + } + type item struct { + node syntax.Node + depth int + } + stack := []item{{root, 0}} + for len(stack) > 0 { + it := stack[len(stack)-1] + stack = stack[:len(stack)-1] + if it.depth > MaxNestingDepth { + return errNestingTooDeep() + } + for _, child := range directChildren(it.node) { + stack = append(stack, item{child, it.depth + 1}) + } + } + return nil +} + +// directChildren returns the immediate child nodes of parent without +// descending further. It uses [syntax.Walk], whose visitor is invoked for the +// node itself first and then for each child; returning true only for parent +// and false for every child stops the walk at depth one, so this call recurses +// a single frame regardless of the overall tree depth. +func directChildren(parent syntax.Node) []syntax.Node { + var children []syntax.Node + syntax.Walk(parent, func(n syntax.Node) bool { + if n == nil { + return false + } + if n == parent { + return true // descend into parent's direct children + } + children = append(children, n) + return false // but no deeper + }) + return children +} diff --git a/interp/nesting_vuln_hunt_test.go b/interp/nesting_vuln_hunt_test.go new file mode 100644 index 000000000..666ed2f50 --- /dev/null +++ b/interp/nesting_vuln_hunt_test.go @@ -0,0 +1,189 @@ +// 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 interp + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" +) + +// Regression coverage for the unbounded-nesting-depth stack-overflow crash +// (VULN-4562 / VULN-1673) and the unbounded-pipeline-depth variant +// (VULN-4535). A sub-MaxScriptBytes but deeply nested script drove the +// recursive-descent parser, the recursive validateNode walk, or the recursive +// executor past the Go runtime's 1 GiB goroutine-stack limit, aborting the +// embedding process with an uncatchable "fatal error: stack overflow" before +// any command, path, mode, or timeout policy was consulted. The fix bounds +// nesting depth with a lexical pre-parse guard (ParseScript) and an iterative +// post-parse guard (validateNode); see MaxNestingDepth. + +// deepNestingScripts returns, for a given per-level repetition count, a set of +// named scripts that each nest one construct to that depth. Parser-recursive +// shapes (brackets and substitutions) are caught by the lexical guard; the +// remaining shapes parse successfully but are caught by the AST guard. +func deepNestingScripts(n int) map[string]string { + r := strings.Repeat + return map[string]string{ + "subshell": r("(", n) + "x" + r(")", n), + "cmd_substitution": r("$(", n) + ":" + r(")", n), + "cmd_subst_in_quote": `"` + r("$(", n) + ":" + r(")", n) + `"`, + "brace_group": r("{ ", n) + "x;" + r(" }", n), + "function_body": r("f() { ", n) + "x;" + r(" }", n), + "param_expansion": "echo " + r("${a:-", n) + "b" + r("}", n), + "arithmetic": "echo $((" + r("(", n) + "1" + r(")", n) + "))", + "pipeline": r("x|", n) + "x", + "logical_and": r("x&&", n) + "x", + "nested_if": r("if x; then ", n) + "x" + r("; fi", n), + "nested_elif": "if x; then x;" + r("elif x; then x;", n) + "fi", + "nested_while": r("while x; do ", n) + "x" + r("; done", n), + "nested_for": r("for a in b; do ", n) + "x" + r("; done", n), + "nested_case": r("case a in a) ", n) + "x" + r(";; esac", n), + "if_inside_quote": `"$(` + r("if x; then ", n) + "x" + r("; fi", n) + `)"`, + } +} + +// TestNestingGuardRejectsDeepScriptsWithoutCrashing checks that moderately +// deep scripts — above MaxNestingDepth but far below the depth that actually +// overflows the stack, so that on unpatched code they would simply parse and +// run rather than crash the test binary — are rejected with a nesting-depth +// error through either ParseScript or Run, never accepted. This fails on main +// (the scripts are accepted) and passes with the fix. +func TestNestingGuardRejectsDeepScriptsWithoutCrashing(t *testing.T) { + const depth = MaxNestingDepth + 1000 + for name, script := range deepNestingScripts(depth) { + t.Run(name, func(t *testing.T) { + prog, err := ParseScript(script, "") + if err != nil { + if !strings.Contains(err.Error(), "nesting exceeds maximum depth") { + t.Fatalf("ParseScript rejected for the wrong reason: %v", err) + } + return // rejected at parse time by the lexical guard + } + // Parsed successfully: the AST guard in validateNode (reached via + // Run) must reject it. + var stdout, stderr bytes.Buffer + runner, err := New(StdIO(nil, &stdout, &stderr)) + if err != nil { + t.Fatalf("New: %v", err) + } + if err := runner.Run(context.Background(), prog); err == nil { + t.Fatalf("Run accepted a script nested to depth %d (stderr=%q)", depth, stderr.String()) + } + if !strings.Contains(stderr.String(), "nesting exceeds maximum depth") { + t.Fatalf("Run did not report the nesting limit; stderr=%q", stderr.String()) + } + }) + } +} + +// TestNestingGuardAllowsRealisticScripts guards against false positives: scripts +// with ordinary — even unusually deep — nesting must still be accepted. +func TestNestingGuardAllowsRealisticScripts(t *testing.T) { + cases := map[string]string{ + "flat": "echo one\necho two\necho three\n", + "nested_cmd_subst": `echo "$(echo "$(echo "$(echo deep)")")"`, + "nested_if": "if true; then if true; then if true; then echo x; fi; fi; fi", + "for_loop": "for i in a b c; do echo $i; done", + "subshell_group": "( ( ( echo x ) ) )", + "and_or": "true && echo a || echo b", + "quoted_parens": `echo "lots of ((((((((( parens ))))))))) in a string"`, + "comment_parens": "echo ok # (((((((((( this is only a comment ))))))))))", + "single_quoted": `echo '((((((((((((((((((((((((((('`, + "moderate_subshell": strings.Repeat("( ", 300) + "echo ok" + strings.Repeat(" )", 300), + "long_pipeline": "echo start" + strings.Repeat(" | cat", 800), + // Keyword-looking words inside quotes must not be counted as compound + // commands, even repeated across a large script. + "keywords_in_double_quotes": strings.Repeat("echo \"waiting for work to do if ready while case\"\n", 8000), + "keywords_in_single_quotes": strings.Repeat("echo 'for do if while until case done fi esac'\n", 8000), + "keyword_substrings": strings.Repeat("echo ifconfig doing format cases fortune\n", 8000), + "brackets_in_double_quotes": strings.Repeat("echo \"json { a: [ ( nested ) ] } done\"\n", 5000), + "real_world": "for f in a b c; do\n if [ -n \"$f\" ]; then\n echo \"item $f ($(basename \"$f\"))\"\n fi\ndone\ncase \"$1\" in start) echo go;; *) echo no;; esac\n", + } + for name, script := range cases { + t.Run(name, func(t *testing.T) { + prog, err := ParseScript(script, "") + if err != nil { + t.Fatalf("ParseScript falsely rejected a realistic script: %v", err) + } + if err := checkNestingDepth(prog); err != nil { + t.Fatalf("AST guard falsely rejected a realistic script: %v", err) + } + }) + } +} + +// TestCheckScriptNestingDepthQuotingContexts pins the lexical guard's handling +// of the contexts in which a bracket cannot open a nested construct. +func TestCheckScriptNestingDepthQuotingContexts(t *testing.T) { + inert := []string{ + "echo '" + strings.Repeat("(", MaxNestingDepth+100) + "'", // single-quoted literal + "echo " + strings.Repeat("\\(", MaxNestingDepth+100), // backslash-escaped + "# " + strings.Repeat("(", MaxNestingDepth+100), // whole-line comment + "echo ok # " + strings.Repeat("(", MaxNestingDepth+100), // trailing comment + } + for i, s := range inert { + if err := checkScriptNestingDepth(s); err != nil { + t.Errorf("case %d: inert brackets must not count toward depth: %v", i, err) + } + } + if err := checkScriptNestingDepth(strings.Repeat("(", MaxNestingDepth+1)); err == nil { + t.Errorf("real bracket nesting past the limit must be rejected") + } +} + +// --- subprocess crash test ------------------------------------------------- + +const deepNestingChildEnv = "RSHELL_DEEP_NESTING_CHILD" + +// TestDeepNestingDoesNotCrashProcess reproduces the real exploit depth — far +// past the stack-overflow threshold on unpatched code — in a re-exec child, so +// that an uncatchable "fatal error: stack overflow" is observed as a failed +// child process rather than aborting this test binary. On main the child dies +// with a runtime-fatal stack overflow; with the fix it returns a normal error +// and exits 0. +func TestDeepNestingDoesNotCrashProcess(t *testing.T) { + if os.Getenv(deepNestingChildEnv) == "1" { + deepNestingChild() + return + } + cmd := exec.Command(os.Args[0], "-test.run=TestDeepNestingDoesNotCrashProcess", "-test.v") + cmd.Env = append(os.Environ(), deepNestingChildEnv+"=1") + out, err := cmd.CombinedOutput() + if bytes.Contains(out, []byte("stack overflow")) { + t.Fatalf("deeply nested script crashed the process with a stack overflow:\n%s", out) + } + if err != nil { + t.Fatalf("deeply nested script crashed the child process: %v\n%s", err, out) + } +} + +// deepNestingChild runs the documented ingestion path (ParseScript then Run) on +// genuinely crash-inducing inputs and exits 0 only if every one was handled +// without a fatal runtime throw. +func deepNestingChild() { + const depth = 300000 // well past the stack-overflow threshold on unpatched code + for name, script := range deepNestingScripts(depth) { + prog, err := ParseScript(script, "") + if err != nil { + continue // rejected by the lexical guard before parsing — fine + } + var stdout, stderr bytes.Buffer + runner, nerr := New(StdIO(nil, &stdout, &stderr)) + if nerr != nil { + fmt.Printf("child: New failed for %s: %v\n", name, nerr) + os.Exit(3) + } + // Must return an error, not crash. + _ = runner.Run(context.Background(), prog) + } + fmt.Println("child: all deep scripts handled without crashing") + os.Exit(0) +} diff --git a/interp/validate.go b/interp/validate.go index 022268e50..ee968e32c 100644 --- a/interp/validate.go +++ b/interp/validate.go @@ -23,6 +23,12 @@ import ( // Brace analysis for tilde validation returns ctx.Err() if ctx is done before // it finishes. func validateNode(ctx context.Context, node syntax.Node, remediationMode bool) error { + // Bound the AST nesting depth before the recursive syntax.Walk below (and + // before any recursive execution) so that neither can overflow the stack. + // See [MaxNestingDepth]. This is iterative and cannot itself overflow. + if err := checkNestingDepth(node); err != nil { + return err + } var err error hdocWords := make(map[*syntax.Word]bool) // literalWords are words that undergo tilde expansion but never brace From d27453d9ea92be562b192fe67f03e89d7d6c04d6 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman <91019033+matt-dz@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:21:15 -0400 Subject: [PATCH 2/3] fix(interp): make nesting pre-scan track parser lexical contexts Review of the first nesting guard found that the pre-parse lexical scan did not bound what it claimed: it treated bytes as structural syntax even where the parser treats them as literal data, so it could both undercount real nesting (letting a deep script reach the recursive parser) and overcount literal text (rejecting valid shallow scripts). Rewrite checkScriptNestingDepth as a quoting- and context-aware state machine over a frame stack whose top selects the mode: - single quotes and quoted heredoc bodies are fully literal; - double quotes and unquoted heredoc bodies are literal except for $(, ${ and backquotes, which still open a nested command context; - reserved words (if/fi/while/done/case/esac/...) and brace command groups ({ ... ; }) are recognised only in command position, so keyword- or brace-looking arguments (echo fi, echo }) are not treated as closers, and a quoted heredoc body cannot clear tracked frames; - an empty "()" is treated as a function-definition parameter list so its body group is still counted. This makes the counter follow the parser's own recursion: it no longer undercounts nesting hidden behind heredoc bodies or argument-position keywords, and no longer rejects literal brackets/keywords in quotes, heredocs or arguments. The post-parse iterative AST guard is unchanged. Regressions: reject deep nesting hidden behind a quoted heredoc body and behind argument-position fi; count deep $( inside an unquoted (expanding) heredoc body; accept literal parens in quoted heredocs/double quotes, keyword/brace-looking arguments, and function definitions. Verified against all 3441 repo scenario scripts (zero false positives) and the full -race suite. Environment: Datadog workspace Co-Authored-By: Claude Opus 4.8 --- interp/nesting.go | 556 +++++++++++++++++++++++-------- interp/nesting_vuln_hunt_test.go | 49 +++ 2 files changed, 457 insertions(+), 148 deletions(-) diff --git a/interp/nesting.go b/interp/nesting.go index 1e82e7a76..5d3d25cc6 100644 --- a/interp/nesting.go +++ b/interp/nesting.go @@ -7,6 +7,7 @@ package interp import ( "fmt" + "strings" "mvdan.cc/sh/v3/syntax" ) @@ -43,202 +44,342 @@ func errNestingTooDeep() error { return fmt.Errorf("script nesting exceeds maximum depth of %d; simplify deeply nested constructs", MaxNestingDepth) } -// Frame kinds tracked by the lexical pre-scan's context stack. +// Frame kinds tracked by the lexical pre-scan's context stack. The top frame +// determines the scanner's mode; each frame also records whether it is a true +// nesting level (counted toward depth) or merely a lexical context. type nestKind uint8 const ( - kindParen nestKind = iota // ( ... ) — subshell, $( ), arithmetic grouping - kindBrace // { ... } — brace group, ${ } - kindBacktick // ` ... ` - kindIf // if ... (closed by fi) - kindElif // elif ... (part of an if ladder, closed by fi) - kindLoop // while/until/for/select ... (closed by done) - kindCase // case ... (closed by esac) - kindSingle // ' ... ' (lexical only, not a nesting level) - kindDouble // " ... " (lexical only, not a nesting level) + kindParen nestKind = iota // ( ... ) / $( ... ) — counted + kindParam // ${ ... } — counted + kindBrace // { ... ; } command group — counted + kindBacktick // ` ... ` — counted + kindIf // if ... (closed by fi) — counted + kindElif // elif ... (part of an if ladder) — counted + kindLoop // while/until/for/select (closed by done) — counted + kindCase // case ... (closed by esac) — counted + kindSingle // ' ... ' — lexical only + kindDouble // " ... " — lexical only + kindHeredocExpand // unquoted heredoc body — lexical only + kindHeredocLiteral // quoted heredoc body — lexical only + kindNone = 0xff // sentinel for "empty stack" ) -// checkScriptNestingDepth is a cheap lexical pre-scan run by [ParseScript] -// before the script text is handed to the recursive-descent parser. Its sole -// job is to stop the parser itself from overflowing the stack on deeply nested -// constructs — subshells and substitutions "(" / "$(" / "${" / "$((", -// backquotes, brace groups "{", and the compound keyword commands if/elif, +// counted reports whether a frame kind is a real nesting level that the parser +// descends into (and therefore must be bounded), as opposed to a quoting or +// heredoc context that only changes how subsequent bytes are interpreted. +func (k nestKind) counted() bool { + switch k { + case kindSingle, kindDouble, kindHeredocExpand, kindHeredocLiteral: + return false + } + return true +} + +type heredocInfo struct { + delim string + stripTabs bool // <<- form: strip leading tabs before comparing the delimiter + expand bool // unquoted delimiter: body still performs $(...)/${...}/`...` expansion +} + +// checkScriptNestingDepth is a lexical pre-scan run by [ParseScript] before the +// script text is handed to the recursive-descent parser. Its job is to stop the +// parser itself from overflowing the stack on deeply nested constructs — +// subshells and substitutions "(" / "$(" / "${" / "$((", backquotes, brace +// command groups "{ … ; }", and the compound keyword commands if/elif, // while/until/for/select, and case — each of which the parser descends into // once per nesting level, before any AST (and therefore the post-parse -// [checkNestingDepth] guard) exists. +// [checkNestingDepth] guard) exists. Pipelines and &&/|| lists are not counted: +// the parser builds those iteratively, so they cannot overflow it; their depth +// is bounded after parsing by [checkNestingDepth]. // -// It is a small quoting-aware state machine over a context stack: the top -// frame determines whether the scanner is in command context (where brackets -// and reserved words open nested constructs), inside single quotes (fully -// literal), or inside double quotes (where only "$(", "${" and backquotes open -// a nested command context — so ordinary words like for/do/if appearing in a -// quoted string are not miscounted). A command substitution opened inside -// double quotes pushes a command-context frame, so reserved words nested -// within it are still counted. +// It is a quoting- and context-aware state machine over a stack of frames. The +// top frame selects the mode, so that bytes are only treated as structural +// syntax where the parser would treat them so: // -// Pipelines and &&/|| lists are deliberately not counted: the parser builds -// those iteratively (no per-stage stack growth), so they cannot overflow it; -// their depth is bounded after parsing by [checkNestingDepth] instead. +// - single quotes and quoted heredoc bodies are fully literal; +// - double quotes and unquoted heredoc bodies are literal except for "$(", +// "${" and backquotes, which still open a nested command context; +// - reserved words (if/fi/while/done/case/esac/…) and brace command groups +// ("{" / "}") are recognised only in command position, so a keyword- or +// brace-looking *argument* (echo fi, echo }) is not mistaken for a closer, +// and heredoc bodies cannot clear the tracked nesting. // -// The scan is a conservative upper bound on the parser's recursion depth — it -// may count a construct the parser would treat as literal — so it can only -// ever reject slightly early, never let a crashing script through. Because the -// threshold is far above any real nesting, this has no practical effect on -// legitimate scripts. +// These contexts make the counter track the parser's own recursion, so the scan +// neither undercounts real nesting (which would let a crashing script through) +// nor overcounts literal text (which would reject a valid script). Unambiguous +// metacharacters — "(", "$(", "${", backquote — are always counted outside the +// literal contexts above; "<<" inside arithmetic is the one accepted +// imprecision (it is treated as a heredoc operator), which can only over-skip +// and never undercount. func checkScriptNestingDepth(script string) error { - stack := make([]nestKind, 0, 64) - depth := 0 // number of stacked frames that are genuine nesting levels + kinds := make([]nestKind, 0, 64) + heredocs := make([]heredocInfo, 0, 4) // parallel stack for heredoc-body frames + var pending []heredocInfo // heredocs declared on the current line, bodies not yet consumed + depth := 0 - top := func() (nestKind, bool) { - if len(stack) == 0 { - return 0, false + top := func() nestKind { + if len(kinds) == 0 { + return kindNone } - return stack[len(stack)-1], true + return kinds[len(kinds)-1] } - push := func(k nestKind) error { - stack = append(stack, k) - if k != kindSingle && k != kindDouble { - depth++ - if depth > MaxNestingDepth { - return errNestingTooDeep() - } + pushCounted := func(k nestKind) error { + kinds = append(kinds, k) + depth++ + if depth > MaxNestingDepth { + return errNestingTooDeep() } return nil } + pushLex := func(k nestKind) { kinds = append(kinds, k) } + pushHeredoc := func(k nestKind, info heredocInfo) { + kinds = append(kinds, k) + heredocs = append(heredocs, info) + } pop := func() { - k := stack[len(stack)-1] - stack = stack[:len(stack)-1] - if k != kindSingle && k != kindDouble { + k := kinds[len(kinds)-1] + kinds = kinds[:len(kinds)-1] + if k == kindHeredocExpand || k == kindHeredocLiteral { + heredocs = heredocs[:len(heredocs)-1] + } else if k.counted() { depth-- } } - // mode reports the lexing context implied by the top frame. - inSingle := func() bool { k, ok := top(); return ok && k == kindSingle } - inDouble := func() bool { k, ok := top(); return ok && k == kindDouble } - - atWordStart := true - isDelim := func(c byte) bool { - switch c { - case ' ', '\t', '\n', '\r', ';', '&', '|', '(', ')', '<', '>', '`': - return true + // startNextHeredoc begins the body of the next pending heredoc, if any and + // if a heredoc body is not already open on top. Bodies are consumed in the + // order declared. + startNextHeredoc := func() { + if len(pending) == 0 { + return + } + if t := top(); t == kindHeredocExpand || t == kindHeredocLiteral { + return + } + info := pending[0] + pending = pending[1:] + if info.expand { + pushHeredoc(kindHeredocExpand, info) + } else { + pushHeredoc(kindHeredocLiteral, info) } - return false } - for i := 0; i < len(script); i++ { - c := script[i] + n := len(script) + i := 0 + lineStart := true + cmdPos := true // next word begins a command (so reserved words are recognised) + + for i < n { + k := top() - if inSingle() { - if c == '\'' { + // Heredoc bodies and delimiter lines are handled at line starts. + if lineStart && (k == kindHeredocExpand || k == kindHeredocLiteral) { + eol := i + for eol < n && script[eol] != '\n' { + eol++ + } + line := script[i:eol] + info := heredocs[len(heredocs)-1] + cmp := line + if info.stripTabs { + cmp = strings.TrimLeft(cmp, "\t") + } + if cmp == info.delim { pop() + i = eol + if i < n { + i++ // consume the newline after the delimiter + } + lineStart = true + startNextHeredoc() + continue } - continue + if k == kindHeredocLiteral { + // Entire body line is literal: skip it. + i = eol + if i < n { + i++ + } + lineStart = true + continue + } + // Expanding body: fall through to per-byte handling of this line, + // which treats it like a double-quoted context (only $(, ${ and + // backquote open nested levels) until the newline. } + lineStart = false - if inDouble() { - switch c { + switch k { + case kindSingle: + if script[i] == '\'' { + pop() + } + i++ + continue + case kindDouble, kindHeredocExpand: + switch script[i] { case '\\': i++ // escape next byte case '"': - pop() + if k == kindDouble { + pop() + } case '`': - if err := push(kindBacktick); err != nil { + if err := pushCounted(kindBacktick); err != nil { return err } + cmdPos = true case '$': - if i+1 < len(script) { - switch script[i+1] { - case '(': - if err := push(kindParen); err != nil { - return err - } - i++ - case '{': - if err := push(kindBrace); err != nil { - return err - } - i++ + if i+1 < n && script[i+1] == '(' { + if err := pushCounted(kindParen); err != nil { + return err + } + i += 2 + cmdPos = true + continue + } + if i+1 < n && script[i+1] == '{' { + if err := pushCounted(kindParam); err != nil { + return err } + i += 2 + continue } + case '\n': + lineStart = true } + i++ continue } - // Command context (base frame or any command-context frame on top). - switch c { - case '\\': - i++ // escape next byte - atWordStart = false - case '\'': - if err := push(kindSingle); err != nil { + // Command context (base, or a command/grouping frame on top). + c := script[i] + switch { + case c == '\\': + i += 2 + cmdPos = false + continue + case c == '\'': + pushLex(kindSingle) + cmdPos = false + case c == '"': + pushLex(kindDouble) + cmdPos = false + case c == '`': + if top() == kindBacktick { + pop() + } else if err := pushCounted(kindBacktick); err != nil { return err } - atWordStart = false - case '"': - if err := push(kindDouble); err != nil { - return err + cmdPos = true + case c == '#' && cmdPosOrSep(script, i): + for i < n && script[i] != '\n' { + i++ } - atWordStart = false - case '`': - if k, ok := top(); ok && k == kindBacktick { - pop() // close the backquote - } else if err := push(kindBacktick); err != nil { - return err + continue + case c == '$': + if i+1 < n && script[i+1] == '(' { + if err := pushCounted(kindParen); err != nil { + return err + } + i += 2 + cmdPos = true + continue } - atWordStart = false - case '#': - if atWordStart { - for i < len(script) && script[i] != '\n' { - i++ + if i+1 < n && script[i+1] == '{' { + if err := pushCounted(kindParam); err != nil { + return err } - atWordStart = true - } else { - atWordStart = false + i += 2 + cmdPos = false + continue } - case '(': - if err := push(kindParen); err != nil { - return err + cmdPos = false + case c == '(': + // An empty "()" (optionally spaced) is a function-definition + // parameter list — the body that follows is a command, so stay in + // command position so that its "{ … ; }" group is counted. (An + // empty subshell matches too; treating what follows as a command + // is harmless.) + j := i + 1 + for j < n && (script[j] == ' ' || script[j] == '\t') { + j++ + } + if j < n && script[j] == ')' { + i = j + 1 + cmdPos = true + continue } - atWordStart = true - case '{': - if err := push(kindBrace); err != nil { + if err := pushCounted(kindParen); err != nil { return err } - atWordStart = true - case ')': - if k, ok := top(); ok && k == kindParen { + cmdPos = true + case c == ')': + if top() == kindParen { pop() } - atWordStart = true - case '}': - if k, ok := top(); ok && k == kindBrace { + cmdPos = false + case c == '{': + if cmdPos && (i+1 >= n || isWordSep(script[i+1])) { + if err := pushCounted(kindBrace); err != nil { + return err + } + cmdPos = true + } else { + cmdPos = false + } + case c == '}': + if top() == kindParam { + pop() + } else if cmdPos && top() == kindBrace { pop() } - atWordStart = true - case ' ', '\t', '\n', '\r', ';', '&', '|': - atWordStart = true - default: - if atWordStart && isASCIILetter(c) { - // Read the maximal run of ASCII letters and decide whether it - // is a standalone reserved word. - j := i + 1 - for j < len(script) && isASCIILetter(script[j]) { - j++ - } - word := script[i:j] - following := byte(' ') - if j < len(script) { - following = script[j] - } - if j == len(script) || isDelim(following) { - if err := applyKeyword(word, &stack, push, pop); err != nil { - return err - } + cmdPos = false + case c == '<': + i = scanRedirOrHeredoc(script, i, &pending) + cmdPos = false + continue + case c == '>': + i++ + if i < n && script[i] == '>' { + i++ + } + cmdPos = false + continue + case c == ';' || c == '&' || c == '|': + i++ + cmdPos = true + continue + case c == '\n': + i++ + cmdPos = true + lineStart = true + startNextHeredoc() + continue + case c == ' ' || c == '\t' || c == '\r': + i++ + continue + case isASCIILetter(c): + j := i + 1 + for j < n && isASCIILetter(script[j]) { + j++ + } + word := script[i:j] + standalone := j >= n || isWordSep(script[j]) + if cmdPos && standalone { + if err := applyKeyword(word, &kinds, &cmdPos, pushCounted, pop); err != nil { + return err } - i = j - 1 // advance past the letter run + } else { + cmdPos = false } - atWordStart = false + i = j + continue + default: + cmdPos = false } + i++ } return nil } @@ -247,36 +388,155 @@ func isASCIILetter(c byte) bool { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') } -// applyKeyword updates the context stack for a standalone reserved word. Only -// the compound-command keywords that make the parser descend a level are -// significant; other words (then, do, in, else, and any non-keyword) are -// ignored. -func applyKeyword(word string, stack *[]nestKind, push func(nestKind) error, pop func()) error { +// isWordSep reports whether c terminates a shell word, so that a letter run +// bounded by separators can be recognised as a standalone reserved word. +func isWordSep(c byte) bool { + switch c { + case ' ', '\t', '\n', '\r', ';', '&', '|', '(', ')', '<', '>', '`': + return true + } + return false +} + +// cmdPosOrSep reports whether the '#' at index i begins a comment: true at the +// start of input or when preceded by an unquoted separator. Being conservative +// here only ever skips genuine comments; a '#' glued to a word (a#b, ${#x}) is +// not treated as a comment. +func cmdPosOrSep(script string, i int) bool { + if i == 0 { + return true + } + switch script[i-1] { + case ' ', '\t', '\n', '\r', ';', '&', '|', '(': + return true + } + return false +} + +// scanRedirOrHeredoc handles a '<' at index i. It recognises "<<-"/"<<" heredoc +// operators (recording the body delimiter in pending so the following lines are +// treated as a heredoc body) and the "<<<" here-string and plain "<" redirects +// (which have no body). It returns the index just past what it consumed. +func scanRedirOrHeredoc(script string, i int, pending *[]heredocInfo) int { + n := len(script) + if i+1 >= n || script[i+1] != '<' { + return i + 1 // plain "<" redirect + } + if i+2 < n && script[i+2] == '<' { + return i + 3 // "<<<" here-string: no heredoc body + } + // Heredoc: "<<" or "<<-". + j := i + 2 + stripTabs := false + if j < n && script[j] == '-' { + stripTabs = true + j++ + } + for j < n && (script[j] == ' ' || script[j] == '\t') { + j++ + } + delim, expand, end := readHeredocDelim(script, j) + if delim != "" { + *pending = append(*pending, heredocInfo{delim: delim, stripTabs: stripTabs, expand: expand}) + } + return end +} + +// readHeredocDelim reads a heredoc delimiter word starting at i, following the +// quoting that decides whether the body expands. A delimiter containing any +// quote or backslash makes the body literal (expand == false); a bare delimiter +// leaves $(...)/${...}/`...` active in the body. It returns the unquoted +// delimiter text, whether the body expands, and the index past the word. +func readHeredocDelim(script string, i int) (string, bool, int) { + n := len(script) + var b strings.Builder + expand := true + for i < n { + c := script[i] + switch c { + case ' ', '\t', '\n', '\r', ';', '&', '|', '<', '>', '(', ')': + return b.String(), expand, i + case '\'': + expand = false + i++ + for i < n && script[i] != '\'' { + b.WriteByte(script[i]) + i++ + } + if i < n { + i++ // closing quote + } + case '"': + expand = false + i++ + for i < n && script[i] != '"' { + b.WriteByte(script[i]) + i++ + } + if i < n { + i++ + } + case '\\': + expand = false + i++ + if i < n { + b.WriteByte(script[i]) + i++ + } + default: + b.WriteByte(c) + i++ + } + } + return b.String(), expand, i +} + +// applyKeyword updates the context stack for a standalone reserved word seen in +// command position. Compound-command openers push a counted frame; their +// closers pop the matching frame; continuation words (then/do/else/in) only +// restore command position for the word that follows. Anything else is an +// ordinary command name. +func applyKeyword(word string, stack *[]nestKind, cmdPos *bool, push func(nestKind) error, pop func()) error { + topIs := func(k nestKind) bool { return len(*stack) > 0 && (*stack)[len(*stack)-1] == k } switch word { case "if": + *cmdPos = true return push(kindIf) case "elif": + *cmdPos = true return push(kindElif) - case "while", "until", "for", "select": + case "while", "until": + *cmdPos = true + return push(kindLoop) + case "for", "select": + *cmdPos = false // followed by a loop variable name, not a command return push(kindLoop) case "case": + *cmdPos = false // followed by the word being matched return push(kindCase) case "fi": // Close an if ladder: pop any elif frames, then the matching if. - for len(*stack) > 0 && (*stack)[len(*stack)-1] == kindElif { + for topIs(kindElif) { pop() } - if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindIf { + if topIs(kindIf) { pop() } + *cmdPos = false case "done": - if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindLoop { + if topIs(kindLoop) { pop() } + *cmdPos = false case "esac": - if len(*stack) > 0 && (*stack)[len(*stack)-1] == kindCase { + if topIs(kindCase) { pop() } + *cmdPos = false + case "then", "do", "else", "in": + *cmdPos = true + default: + *cmdPos = false // ordinary command name } return nil } diff --git a/interp/nesting_vuln_hunt_test.go b/interp/nesting_vuln_hunt_test.go index 666ed2f50..767e05304 100644 --- a/interp/nesting_vuln_hunt_test.go +++ b/interp/nesting_vuln_hunt_test.go @@ -106,6 +106,15 @@ func TestNestingGuardAllowsRealisticScripts(t *testing.T) { "keyword_substrings": strings.Repeat("echo ifconfig doing format cases fortune\n", 8000), "brackets_in_double_quotes": strings.Repeat("echo \"json { a: [ ( nested ) ] } done\"\n", 5000), "real_world": "for f in a b c; do\n if [ -n \"$f\" ]; then\n echo \"item $f ($(basename \"$f\"))\"\n fi\ndone\ncase \"$1\" in start) echo go;; *) echo no;; esac\n", + // Context-tracking false-positive regressions (reported by review of the + // first version of the lexical guard): literal brackets/keywords that the + // parser treats as data must not be counted as nesting. + "literal_parens_in_quoted_heredoc": "cat <<'E'\n" + strings.Repeat("(", 5100) + "\nE\n", + "keyword_looking_arguments": strings.Repeat("echo if\n", 5001), + "closer_looking_arguments": strings.Repeat("echo fi done esac\n", 5001), + "brace_looking_arguments": strings.Repeat("echo }\n", 5001), + "function_definitions": strings.Repeat("f() { echo hi; }\n", 4000), + "normal_heredoc_with_expansion": "cat < Date: Wed, 28 Oct 2026 17:13:57 -0400 Subject: [PATCH 3/3] fix(interp): close remaining lexical nesting-guard undercounts Address the second-round review of the pre-parse nesting guard. The scanner now follows the parser's lexical contexts for the shapes that still let deep nesting through: - "<<" inside arithmetic ($((..)), ((..)), $[..], let arguments, and the a[..] index of assignments and declaration builtins) is a shift, so it no longer opens a phantom heredoc body; ${..} words are scanned as words. - A case pattern list is scanned as patterns, and its ")" puts the scanner in command position, so case and { .. } in case bodies are counted. - Heredoc delimiter lines are compared with a trailing CR removed (CRLF). - then/else/do restore command position only inside their own compound (fixes the "then in if" false positive). - Command position is also kept after "!", "time [-p]", "function NAME", "coproc NAME", and backslash-newline; $'..' honours backslash escapes. Adds regressions for each bypass shape (rejected before parsing, and not crashing an 8 MiB-stack child), shallow variants, and keyword-looking words that must still be accepted. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5.5 --- interp/nesting.go | 1113 +++++++++++++++++++++--------- interp/nesting_vuln_hunt_test.go | 130 +++- 2 files changed, 918 insertions(+), 325 deletions(-) diff --git a/interp/nesting.go b/interp/nesting.go index 5d3d25cc6..29b742fd1 100644 --- a/interp/nesting.go +++ b/interp/nesting.go @@ -50,30 +50,41 @@ func errNestingTooDeep() error { type nestKind uint8 const ( - kindParen nestKind = iota // ( ... ) / $( ... ) — counted + kindParen nestKind = iota // ( ... ) / $( ... ) / <( ... ) — counted kindParam // ${ ... } — counted kindBrace // { ... ; } command group — counted kindBacktick // ` ... ` — counted kindIf // if ... (closed by fi) — counted kindElif // elif ... (part of an if ladder) — counted kindLoop // while/until/for/select (closed by done) — counted - kindCase // case ... (closed by esac) — counted + kindCase // case ... (closed by esac); on top: inside a case body — counted + kindArith // $(( ... )) / (( ... )) — counted + kindArithSq // $[ ... ] — counted + kindArithParen // ( ... ) inside arithmetic — counted + kindArithIndex // [ ... ] inside arithmetic, or an assignment's a[ ... ] index — counted + kindPatGroup // ( ... ) extglob group inside a case pattern — counted kindSingle // ' ... ' — lexical only + kindAnsi // $' ... ' — lexical only kindDouble // " ... " — lexical only kindHeredocExpand // unquoted heredoc body — lexical only kindHeredocLiteral // quoted heredoc body — lexical only + kindCaseHead0 // after "case", before the subject word — lexical only + kindCaseHead1 // after the case subject, before "in" — lexical only + kindCasePat // a case pattern list, up to its ")" — lexical only + kindLet // the arithmetic arguments of "let" — lexical only kindNone = 0xff // sentinel for "empty stack" ) // counted reports whether a frame kind is a real nesting level that the parser -// descends into (and therefore must be bounded), as opposed to a quoting or -// heredoc context that only changes how subsequent bytes are interpreted. +// descends into (and therefore must be bounded), as opposed to a quoting, +// heredoc, or grammar-position context that only changes how subsequent bytes +// are interpreted. func (k nestKind) counted() bool { - switch k { - case kindSingle, kindDouble, kindHeredocExpand, kindHeredocLiteral: - return false - } - return true + return k < kindSingle +} + +func (k nestKind) isHeredoc() bool { + return k == kindHeredocExpand || k == kindHeredocLiteral } type heredocInfo struct { @@ -85,309 +96,813 @@ type heredocInfo struct { // checkScriptNestingDepth is a lexical pre-scan run by [ParseScript] before the // script text is handed to the recursive-descent parser. Its job is to stop the // parser itself from overflowing the stack on deeply nested constructs — -// subshells and substitutions "(" / "$(" / "${" / "$((", backquotes, brace -// command groups "{ … ; }", and the compound keyword commands if/elif, -// while/until/for/select, and case — each of which the parser descends into -// once per nesting level, before any AST (and therefore the post-parse -// [checkNestingDepth] guard) exists. Pipelines and &&/|| lists are not counted: -// the parser builds those iteratively, so they cannot overflow it; their depth -// is bounded after parsing by [checkNestingDepth]. +// subshells and substitutions "(" / "$(" / "${" / "$((" / "$[", backquotes, +// brace command groups "{ … ; }", arithmetic parentheses and indexes, and the +// compound keyword commands if/elif, while/until/for/select, and case — each of +// which the parser descends into once per nesting level, before any AST (and +// therefore the post-parse [checkNestingDepth] guard) exists. Pipelines and +// &&/|| lists are not counted: the parser builds those iteratively, so they +// cannot overflow it; their depth is bounded after parsing by +// [checkNestingDepth]. // // It is a quoting- and context-aware state machine over a stack of frames. The // top frame selects the mode, so that bytes are only treated as structural -// syntax where the parser would treat them so: +// syntax where mvdan.cc/sh's lexer would treat them so: // -// - single quotes and quoted heredoc bodies are fully literal; -// - double quotes and unquoted heredoc bodies are literal except for "$(", -// "${" and backquotes, which still open a nested command context; +// - single quotes and quoted heredoc bodies are fully literal; $'…' strings +// are literal apart from backslash escapes; +// - double quotes, unquoted heredoc bodies, and ${…} words are literal except +// for "$(", "${", "$((", "$[" and backquotes, which still open nested +// contexts (${…} additionally honours quotes); +// - arithmetic — "$((…))", "((…))", "$[…]", "let" arguments, and the a[…] +// index of an assignment — treats "<<" as a shift and keywords as plain +// names, so neither can open a phantom heredoc or compound command; // - reserved words (if/fi/while/done/case/esac/…) and brace command groups -// ("{" / "}") are recognised only in command position, so a keyword- or -// brace-looking *argument* (echo fi, echo }) is not mistaken for a closer, -// and heredoc bodies cannot clear the tracked nesting. +// ("{" / "}") are recognised only in command position, including after +// "!", "time", a case pattern's ")" and a function name, so a keyword- or +// brace-looking *argument* (echo fi, echo }) is not mistaken for a closer; +// continuation words (then/else/do) restore command position only inside +// the compound they belong to; +// - case pattern lists are scanned as patterns, not commands; +// - heredoc delimiter lines are matched with a trailing CR removed, as the +// parser does for CRLF scripts. // -// These contexts make the counter track the parser's own recursion, so the scan -// neither undercounts real nesting (which would let a crashing script through) -// nor overcounts literal text (which would reject a valid script). Unambiguous -// metacharacters — "(", "$(", "${", backquote — are always counted outside the -// literal contexts above; "<<" inside arithmetic is the one accepted -// imprecision (it is treated as a heredoc operator), which can only over-skip -// and never undercount. +// The invariant that matters for safety is that the scan never undercounts: +// any byte the parser lexes as structure must not be skipped as literal text, +// and literal text must not be taken for a closer. Every context above exists +// because getting it wrong in one direction or the other lets a deeply nested +// script through (undercount) or rejects a valid one (overcount). func checkScriptNestingDepth(script string) error { - kinds := make([]nestKind, 0, 64) - heredocs := make([]heredocInfo, 0, 4) // parallel stack for heredoc-body frames - var pending []heredocInfo // heredocs declared on the current line, bodies not yet consumed - depth := 0 + sc := nestScanner{s: script, lineStart: true} + sc.startCmd() + for sc.i < len(sc.s) { + if err := sc.step(); err != nil { + return err + } + } + return nil +} + +// nestScanner holds the lexical pre-scan state; see [checkScriptNestingDepth]. +type nestScanner struct { + s string + i int + kinds []nestKind + depth int + + heredocs []heredocInfo // parallel stack for heredoc-body frames + pending []heredocInfo // heredocs declared on the current line, bodies not yet consumed + + lineStart bool // at the start of a line (heredoc delimiter lines are matched here) + + // Command-context state. + cmdPos bool // next word begins a command (so reserved words are recognised) + wordStart bool // next non-separator byte begins a new word + argWords int // non-assignment, non-redirect words seen in the current simple command + inDecl bool // current simple command is declare/local/export/readonly/typeset/nameref + redirTarget bool // next word is a redirect target + afterTime bool // previous word was the "time" keyword (so "-p" keeps command position) + + patStart bool // in a case pattern list, nothing of the current pattern consumed yet + prevSpace bool // in arithmetic, the previous byte was a blank (ends a "let" argument list before an operator) +} + +func (sc *nestScanner) top() nestKind { + if len(sc.kinds) == 0 { + return kindNone + } + return sc.kinds[len(sc.kinds)-1] +} + +func (sc *nestScanner) push(k nestKind) error { + sc.kinds = append(sc.kinds, k) + if !k.counted() { + return nil + } + sc.depth++ + if sc.depth > MaxNestingDepth { + return errNestingTooDeep() + } + return nil +} + +func (sc *nestScanner) replaceTop(k nestKind) { sc.kinds[len(sc.kinds)-1] = k } + +func (sc *nestScanner) pop() { + k := sc.kinds[len(sc.kinds)-1] + sc.kinds = sc.kinds[:len(sc.kinds)-1] + if k.isHeredoc() { + sc.heredocs = sc.heredocs[:len(sc.heredocs)-1] + } else if k.counted() { + sc.depth-- + } +} + +// startCmd records that the next word begins a new simple command. +func (sc *nestScanner) startCmd() { + sc.cmdPos = true + sc.wordStart = true + sc.argWords = 0 + sc.inDecl = false + sc.redirTarget = false + sc.afterTime = false +} - top := func() nestKind { - if len(kinds) == 0 { - return kindNone +// startNextHeredoc begins the body of the next pending heredoc, if any and if a +// heredoc body is not already open on top. Bodies are consumed in the order +// declared. +func (sc *nestScanner) startNextHeredoc() { + if len(sc.pending) == 0 || sc.top().isHeredoc() { + return + } + info := sc.pending[0] + sc.pending = sc.pending[1:] + k := kindHeredocLiteral + if info.expand { + k = kindHeredocExpand + } + sc.kinds = append(sc.kinds, k) + sc.heredocs = append(sc.heredocs, info) +} + +func (sc *nestScanner) peek(off int) byte { + if j := sc.i + off; j < len(sc.s) { + return sc.s[j] + } + return 0 +} + +// step consumes at least one byte (or changes the frame stack) according to the +// mode selected by the top frame. +func (sc *nestScanner) step() error { + k := sc.top() + if sc.lineStart && k.isHeredoc() { + if sc.heredocLine(k) { + return nil } - return kinds[len(kinds)-1] + // Expanding body line: fall through to per-byte handling, which + // treats it like a double-quoted context until the newline. } - pushCounted := func(k nestKind) error { - kinds = append(kinds, k) - depth++ - if depth > MaxNestingDepth { - return errNestingTooDeep() + sc.lineStart = false + switch k { + case kindSingle: + if sc.s[sc.i] == '\'' { + sc.pop() } + sc.i++ return nil + case kindAnsi: + switch sc.s[sc.i] { + case '\\': + sc.i++ + case '\'': + sc.pop() + } + sc.i++ + return nil + case kindDouble, kindHeredocExpand: + return sc.stepDouble(k) + case kindParam: + return sc.stepParam() + case kindArith, kindArithSq, kindArithParen, kindArithIndex, kindLet: + return sc.stepArith(k) + case kindCasePat, kindPatGroup: + return sc.stepPattern(k) } - pushLex := func(k nestKind) { kinds = append(kinds, k) } - pushHeredoc := func(k nestKind, info heredocInfo) { - kinds = append(kinds, k) - heredocs = append(heredocs, info) - } - pop := func() { - k := kinds[len(kinds)-1] - kinds = kinds[:len(kinds)-1] - if k == kindHeredocExpand || k == kindHeredocLiteral { - heredocs = heredocs[:len(heredocs)-1] - } else if k.counted() { - depth-- - } - } - // startNextHeredoc begins the body of the next pending heredoc, if any and - // if a heredoc body is not already open on top. Bodies are consumed in the - // order declared. - startNextHeredoc := func() { - if len(pending) == 0 { - return - } - if t := top(); t == kindHeredocExpand || t == kindHeredocLiteral { - return - } - info := pending[0] - pending = pending[1:] - if info.expand { - pushHeredoc(kindHeredocExpand, info) - } else { - pushHeredoc(kindHeredocLiteral, info) + return sc.stepCommand(k) +} + +// heredocLine handles a heredoc body line starting at sc.i. It reports whether +// it consumed the line (a delimiter line, or any line of a literal body). +func (sc *nestScanner) heredocLine(k nestKind) bool { + eol := strings.IndexByte(sc.s[sc.i:], '\n') + if eol < 0 { + eol = len(sc.s) + } else { + eol += sc.i + } + info := sc.heredocs[len(sc.heredocs)-1] + line := sc.s[sc.i:eol] + if len(line) > 0 && line[len(line)-1] == '\r' { + line = line[:len(line)-1] // the parser closes "E\r\n" in CRLF scripts + } + if info.stripTabs { + line = strings.TrimLeft(line, "\t") + } + consume := func() { + sc.i = eol + if sc.i < len(sc.s) { + sc.i++ // the newline } + sc.lineStart = true + } + if line == info.delim { + sc.pop() + consume() + sc.startNextHeredoc() + return true + } + if k == kindHeredocLiteral { + consume() + return true } + return false +} - n := len(script) - i := 0 - lineStart := true - cmdPos := true // next word begins a command (so reserved words are recognised) +// dollar handles a '$' at sc.i in any context where expansions are active. +// ansi reports whether $'…' is an ANSI-C string there (it is not inside double +// quotes or heredoc bodies). +func (sc *nestScanner) dollar(ansi bool) error { + switch sc.peek(1) { + case '(': + if sc.peek(2) == '(' { + sc.i += 3 + return sc.push(kindArith) + } + sc.i += 2 + sc.startCmd() + return sc.push(kindParen) + case '{': + sc.i += 2 + return sc.push(kindParam) + case '[': + sc.i += 2 + return sc.push(kindArithSq) + case '\'': + if ansi { + sc.i += 2 + return sc.push(kindAnsi) + } + } + sc.i++ + return nil +} - for i < n { - k := top() +func (sc *nestScanner) backtick() error { + sc.i++ + sc.startCmd() + return sc.push(kindBacktick) +} - // Heredoc bodies and delimiter lines are handled at line starts. - if lineStart && (k == kindHeredocExpand || k == kindHeredocLiteral) { - eol := i - for eol < n && script[eol] != '\n' { - eol++ - } - line := script[i:eol] - info := heredocs[len(heredocs)-1] - cmp := line - if info.stripTabs { - cmp = strings.TrimLeft(cmp, "\t") - } - if cmp == info.delim { - pop() - i = eol - if i < n { - i++ // consume the newline after the delimiter - } - lineStart = true - startNextHeredoc() - continue - } - if k == kindHeredocLiteral { - // Entire body line is literal: skip it. - i = eol - if i < n { - i++ - } - lineStart = true - continue - } - // Expanding body: fall through to per-byte handling of this line, - // which treats it like a double-quoted context (only $(, ${ and - // backquote open nested levels) until the newline. +// stepDouble scans a double-quoted string or an unquoted heredoc body: only +// substitutions open nested contexts. +func (sc *nestScanner) stepDouble(k nestKind) error { + switch sc.s[sc.i] { + case '\\': + sc.i++ // escape next byte (a backslash-newline joins lines) + case '"': + if k == kindDouble { + sc.pop() } - lineStart = false + case '`': + return sc.backtick() + case '$': + return sc.dollar(false) + case '\n': + sc.lineStart = true + } + sc.i++ + return nil +} - switch k { - case kindSingle: - if script[i] == '\'' { - pop() - } - i++ - continue - case kindDouble, kindHeredocExpand: - switch script[i] { - case '\\': - i++ // escape next byte - case '"': - if k == kindDouble { - pop() - } - case '`': - if err := pushCounted(kindBacktick); err != nil { - return err - } - cmdPos = true - case '$': - if i+1 < n && script[i+1] == '(' { - if err := pushCounted(kindParen); err != nil { - return err - } - i += 2 - cmdPos = true - continue - } - if i+1 < n && script[i+1] == '{' { - if err := pushCounted(kindParam); err != nil { - return err - } - i += 2 - continue - } - case '\n': - lineStart = true - } - i++ - continue - } +// stepParam scans the word inside ${…}: everything is literal (including +// "(", "{", "<<" and keywords) except quotes, substitutions, and the closing +// brace. The parser does not nest bare braces here: ${x:-{}} ends at the first +// "}". +func (sc *nestScanner) stepParam() error { + switch sc.s[sc.i] { + case '}': + sc.pop() + case '\\': + sc.i++ + case '\'': + sc.i++ + return sc.push(kindSingle) + case '"': + sc.i++ + return sc.push(kindDouble) + case '`': + return sc.backtick() + case '$': + return sc.dollar(true) + } + sc.i++ + return nil +} - // Command context (base, or a command/grouping frame on top). - c := script[i] +// stepArith scans arithmetic: "<<" is a shift, words are variable names, and +// only parentheses, brackets, quotes and substitutions change context. A "let" +// argument list additionally ends at a command terminator, or at an operator +// or redirect that follows a blank (the parser reads "let x && y" as a list +// but "let x&&y" as one expression). +func (sc *nestScanner) stepArith(k nestKind) error { + c := sc.s[sc.i] + if k == kindLet { switch { - case c == '\\': - i += 2 - cmdPos = false - continue - case c == '\'': - pushLex(kindSingle) - cmdPos = false - case c == '"': - pushLex(kindDouble) - cmdPos = false - case c == '`': - if top() == kindBacktick { - pop() - } else if err := pushCounted(kindBacktick); err != nil { - return err - } - cmdPos = true - case c == '#' && cmdPosOrSep(script, i): - for i < n && script[i] != '\n' { - i++ - } - continue - case c == '$': - if i+1 < n && script[i+1] == '(' { - if err := pushCounted(kindParen); err != nil { - return err - } - i += 2 - cmdPos = true - continue - } - if i+1 < n && script[i+1] == '{' { - if err := pushCounted(kindParam); err != nil { - return err - } - i += 2 - cmdPos = false - continue + case c == ';' || c == '\n' || c == ')' || c == '`', + sc.prevSpace && (c == '&' || c == '|' || c == '<' || c == '>'): + sc.pop() // reprocess c in the enclosing context + sc.cmdPos = false + return nil + case c == ' ' || c == '\t' || c == '\r': + sc.prevSpace = true + sc.i++ + return nil + } + } + sc.prevSpace = false + switch c { + case '(': + sc.i++ + return sc.push(kindArithParen) + case ')': + switch { + case k == kindArithParen: + sc.pop() + case k == kindArith && sc.peek(1) == ')': + sc.pop() + sc.i++ + sc.cmdPos = false + case k == kindArith: + sc.pop() // malformed; the parser stops here + } + case '[': + sc.i++ + return sc.push(kindArithIndex) + case ']': + if k == kindArithIndex || k == kindArithSq { + sc.pop() + } + case '\\': + sc.i++ + case '\'': + sc.i++ + return sc.push(kindSingle) + case '"': + sc.i++ + return sc.push(kindDouble) + case '`': + return sc.backtick() + case '$': + return sc.dollar(true) + } + sc.i++ + return nil +} + +// stepPattern scans a case pattern list ("a|b)") or an extglob group inside +// one. Patterns are words, not commands: reserved words are not recognised, +// except a leading "esac" that closes the case statement. The ")" that ends +// the list puts the scanner in command position for the case body. +func (sc *nestScanner) stepPattern(k nestKind) error { + c := sc.s[sc.i] + switch { + case c == ' ' || c == '\t' || c == '\r': + sc.i++ + return nil + case c == '\n': + sc.i++ + sc.lineStart = true + sc.startNextHeredoc() + return nil + case c == '#' && cmdPosOrSep(sc.s, sc.i): + sc.skipComment() + return nil + case isASCIILetter(c): + j := sc.i + 1 + for j < len(sc.s) && isASCIILetter(sc.s[j]) { + j++ + } + if k == kindCasePat && sc.patStart && sc.s[sc.i:j] == "esac" && (j >= len(sc.s) || isWordSep(sc.s[j])) { + sc.pop() // pattern list + if sc.top() == kindCase { + sc.pop() } - cmdPos = false - case c == '(': - // An empty "()" (optionally spaced) is a function-definition - // parameter list — the body that follows is a command, so stay in - // command position so that its "{ … ; }" group is counted. (An - // empty subshell matches too; treating what follows as a command - // is harmless.) - j := i + 1 - for j < n && (script[j] == ' ' || script[j] == '\t') { - j++ + sc.startCmd() + sc.cmdPos = false + } + sc.patStart = false + sc.i = j + return nil + } + start := sc.patStart + sc.patStart = false + switch c { + case '(': + sc.i++ + if k == kindCasePat && start { + return nil // optional leading "(" of a pattern list + } + return sc.push(kindPatGroup) + case ')': + sc.i++ + sc.pop() + if k == kindCasePat { + sc.startCmd() // case body + } + return nil + case '\\': + sc.i += 2 + return nil + case '\'': + sc.i++ + return sc.push(kindSingle) + case '"': + sc.i++ + return sc.push(kindDouble) + case '`': + return sc.backtick() + case '$': + return sc.dollar(true) + } + sc.i++ + return nil +} + +func (sc *nestScanner) skipComment() { + for sc.i < len(sc.s) && sc.s[sc.i] != '\n' { + sc.i++ + } +} + +// stepCommand scans command context: the top level, or a frame whose body is +// a command list (subshell, command substitution, brace group, backquotes, +// compound command, case body), or a case statement's head. +func (sc *nestScanner) stepCommand(k nestKind) error { + c := sc.s[sc.i] + head := k == kindCaseHead0 || k == kindCaseHead1 + subjectSeen := k == kindCaseHead1 // "in" is only the keyword after the subject word + if head { + sc.cmdPos = false // the case subject is a word, not a command + if k == kindCaseHead0 && !isBlank(c) && c != '\n' { + sc.replaceTop(kindCaseHead1) + k = kindCaseHead1 + } + } + + switch c { + case ' ', '\t', '\r': + sc.i++ + sc.wordStart = true + return nil + case '\n': + sc.i++ + sc.startCmd() + sc.lineStart = true + sc.startNextHeredoc() + return nil + case ';': + if k == kindCase && (sc.peek(1) == ';' || sc.peek(1) == '&') { + // ";;", ";;&" or ";&" ends a case item; a pattern list follows. + sc.i += 2 + if sc.peek(0) == '&' { + sc.i++ } - if j < n && script[j] == ')' { - i = j + 1 - cmdPos = true - continue + sc.patStart = true + return sc.push(kindCasePat) + } + sc.i++ + sc.startCmd() + return nil + case '&': + if sc.peek(1) == '>' { // &> / &>> redirect + sc.i += 2 + if sc.peek(0) == '>' { + sc.i++ } - if err := pushCounted(kindParen); err != nil { + sc.redirTarget = true + sc.wordStart = true + sc.cmdPos = false + return nil + } + sc.i++ + sc.startCmd() + return nil + case '|': + sc.i++ + sc.startCmd() + return nil + case '<', '>': + if sc.peek(1) == '(' { // process substitution + sc.i += 2 + sc.startCmd() + return sc.push(kindParen) + } + if c == '<' { + sc.i = scanRedirOrHeredoc(sc.s, sc.i, &sc.pending) + } else { + sc.i++ + } + if sc.i < len(sc.s) && (sc.s[sc.i] == '>' || sc.s[sc.i] == '&' || sc.s[sc.i] == '|') { + sc.i++ // >>, >&, <&, >| + } + sc.redirTarget = true + sc.wordStart = true + sc.cmdPos = false + return nil + case '(': + sc.wordStart = true + // An empty "()" (optionally spaced) is a function-definition + // parameter list — the body that follows is a command, so stay in + // command position so that its "{ … ; }" group is counted. + j := sc.i + 1 + for j < len(sc.s) && isBlank(sc.s[j]) { + j++ + } + if j < len(sc.s) && sc.s[j] == ')' { + sc.i = j + 1 + sc.startCmd() + return nil + } + if sc.peek(1) == '(' { // the parser always reads "((" as arithmetic + sc.i += 2 + sc.cmdPos = false + return sc.push(kindArith) + } + sc.i++ + sc.startCmd() + return sc.push(kindParen) + case ')': + if k == kindParen { + sc.pop() + } + sc.i++ + sc.wordStart = true + sc.cmdPos = false + return nil + } + + if sc.wordStart { + sc.wordStart = false + if c == '!' && sc.cmdPos && (sc.i+1 >= len(sc.s) || isWordSep(sc.s[sc.i+1])) { + sc.i++ // pipeline negation: a command still follows + sc.wordStart = true + return nil + } + if !head { + if handled, err := sc.beginWord(); handled || err != nil { return err } - cmdPos = true - case c == ')': - if top() == kindParen { - pop() - } - cmdPos = false - case c == '{': - if cmdPos && (i+1 >= n || isWordSep(script[i+1])) { - if err := pushCounted(kindBrace); err != nil { - return err - } - cmdPos = true - } else { - cmdPos = false - } - case c == '}': - if top() == kindParam { - pop() - } else if cmdPos && top() == kindBrace { - pop() - } - cmdPos = false - case c == '<': - i = scanRedirOrHeredoc(script, i, &pending) - cmdPos = false - continue - case c == '>': - i++ - if i < n && script[i] == '>' { - i++ - } - cmdPos = false - continue - case c == ';' || c == '&' || c == '|': - i++ - cmdPos = true - continue - case c == '\n': - i++ - cmdPos = true - lineStart = true - startNextHeredoc() - continue - case c == ' ' || c == '\t' || c == '\r': - i++ - continue - case isASCIILetter(c): - j := i + 1 - for j < n && isASCIILetter(script[j]) { - j++ + } + } + + switch { + case c == '\\': + if sc.peek(1) == '\n' { + sc.i += 2 // line continuation: does not end the word or change position + return nil + } + sc.i += 2 + sc.cmdPos = false + return nil + case c == '\'': + sc.i++ + sc.cmdPos = false + return sc.push(kindSingle) + case c == '"': + sc.i++ + sc.cmdPos = false + return sc.push(kindDouble) + case c == '`': + if k == kindBacktick { + sc.pop() + sc.i++ + sc.cmdPos = false + return nil + } + return sc.backtick() + case c == '#' && cmdPosOrSep(sc.s, sc.i): + sc.skipComment() + return nil + case c == '$': + sc.cmdPos = false + return sc.dollar(true) + case c == '{': + sc.i++ + if sc.cmdPos && (sc.i >= len(sc.s) || isWordSep(sc.s[sc.i])) { + sc.startCmd() + return sc.push(kindBrace) + } + sc.cmdPos = false + return nil + case c == '}': + sc.i++ + if sc.cmdPos && k == kindBrace && (sc.i >= len(sc.s) || isWordSep(sc.s[sc.i])) { + sc.pop() + } + sc.cmdPos = false + return nil + case isASCIILetter(c): + j := sc.i + 1 + for j < len(sc.s) && isASCIILetter(sc.s[j]) { + j++ + } + word := sc.s[sc.i:j] + standalone := j >= len(sc.s) || isWordSep(sc.s[j]) + sc.i = j + if head { + if subjectSeen && word == "in" && standalone { + sc.replaceTop(kindCasePat) + sc.patStart = true } - word := script[i:j] - standalone := j >= n || isWordSep(script[j]) - if cmdPos && standalone { - if err := applyKeyword(word, &kinds, &cmdPos, pushCounted, pop); err != nil { - return err - } - } else { - cmdPos = false + return nil + } + if sc.cmdPos && standalone { + return sc.keyword(word) + } + sc.cmdPos = false + return nil + } + sc.i++ + sc.cmdPos = false + return nil +} + +// beginWord classifies the word starting at sc.i in command context. Words +// that the parser lexes as assignments with an a[…] index — the prefix +// assignments of a simple command, and every argument of a declaration +// builtin — have that index parsed as arithmetic, so it is scanned as such +// ("<<" there is a shift, not a heredoc). It reports whether it consumed +// input. +func (sc *nestScanner) beginWord() (bool, error) { + if sc.redirTarget { + sc.redirTarget = false + return false, nil + } + afterTime := sc.afterTime + sc.afterTime = false + j := sc.i + for j < len(sc.s) && (isASCIILetter(sc.s[j]) || sc.s[j] == '_' || (j > sc.i && isASCIIDigit(sc.s[j]))) { + j++ + } + name := sc.s[sc.i:j] + next := byte(0) + if j < len(sc.s) { + next = sc.s[j] + } + switch { + case name != "" && next == '[' && (sc.argWords == 0 || sc.inDecl): + sc.i = j + 1 + sc.cmdPos = false + return true, sc.push(kindArithIndex) + case name != "" && (next == '=' || (next == '+' && j+1 < len(sc.s) && sc.s[j+1] == '=')): + return false, nil // assignment + case name == "" && isASCIIDigit(sc.s[sc.i]): + d := sc.i + for d < len(sc.s) && isASCIIDigit(sc.s[d]) { + d++ + } + if d < len(sc.s) && (sc.s[d] == '<' || sc.s[d] == '>') { + return false, nil // file-descriptor number of a redirect + } + case afterTime && sc.cmdPos && strings.HasPrefix(sc.s[sc.i:], "-p") && + (sc.i+2 >= len(sc.s) || isWordSep(sc.s[sc.i+2])): + sc.i += 2 // time -p: a command still follows + sc.wordStart = true + return true, nil + } + sc.argWords++ + if sc.argWords == 1 && sc.cmdPos { + switch name { + case "declare", "local", "export", "readonly", "typeset", "nameref": + if next == 0 || isWordSep(next) { + sc.inDecl = true } - i = j - continue - default: - cmdPos = false } - i++ + } + return false, nil +} + +// keyword updates the context stack for a standalone word seen in command +// position. Compound-command openers push a counted frame; their closers pop +// the matching frame; continuation words (then/else/do) restore command +// position only inside the compound they belong to. Anything else is an +// ordinary command name. +func (sc *nestScanner) keyword(word string) error { + top := sc.top() + switch word { + case "if": + sc.startCmd() + return sc.push(kindIf) + case "elif": + sc.startCmd() + return sc.push(kindElif) + case "while", "until": + sc.startCmd() + return sc.push(kindLoop) + case "for", "select": + sc.cmdPos = false // followed by a loop variable name, not a command + return sc.push(kindLoop) + case "case": + sc.cmdPos = false // followed by the word being matched + if err := sc.push(kindCase); err != nil { + return err + } + return sc.push(kindCaseHead0) + case "fi": + // Close an if ladder: pop any elif frames, then the matching if. + for sc.top() == kindElif { + sc.pop() + } + if sc.top() == kindIf { + sc.pop() + } + sc.cmdPos = false + case "done": + if top == kindLoop { + sc.pop() + } + sc.cmdPos = false + case "esac": + if top == kindCase { + sc.pop() + } + sc.cmdPos = false + case "then", "else": + if top == kindIf || top == kindElif { + sc.startCmd() + } else { + sc.cmdPos = false + } + case "do": + if top == kindLoop { + sc.startCmd() + } else { + sc.cmdPos = false + } + case "time": + sc.startCmd() + sc.afterTime = true + case "function", "coproc": + // function NAME [()] BODY / coproc [NAME] COMMAND: skip the name + // so the body is scanned in command position. (For coproc the + // first word may instead be the command itself; skipping it only + // ever over-counts a keyword-looking argument.) + j := sc.i + for j < len(sc.s) && isBlank(sc.s[j]) { + j++ + } + if word == "coproc" && !coprocNamed(sc.s, j) { + sc.startCmd() + return nil + } + for j < len(sc.s) && !isWordSep(sc.s[j]) { + j++ + } + sc.i = j + sc.startCmd() + case "let": + sc.cmdPos = false + sc.prevSpace = false + return sc.push(kindLet) + default: + sc.cmdPos = false // ordinary command name } return nil } +// coprocNamed reports whether the coproc at script[j:] is "coproc NAME +// COMPOUND", i.e. a plain word followed by a compound command opener. +func coprocNamed(script string, j int) bool { + k := j + for k < len(script) && !isWordSep(script[k]) { + k++ + } + if k == j { + return false + } + switch script[j:k] { + case "if", "while", "until", "for", "select", "case", "{": + return false + } + for k < len(script) && isBlank(script[k]) { + k++ + } + if k >= len(script) { + return false + } + if script[k] == '(' || script[k] == '{' { + return true + } + e := k + for e < len(script) && isASCIILetter(script[e]) { + e++ + } + switch script[k:e] { + case "if", "while", "until", "for", "select", "case": + return true + } + return false +} + func isASCIILetter(c byte) bool { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') } +func isASCIIDigit(c byte) bool { return c >= '0' && c <= '9' } + +func isBlank(c byte) bool { return c == ' ' || c == '\t' || c == '\r' } + // isWordSep reports whether c terminates a shell word, so that a letter run // bounded by separators can be recognised as a standalone reserved word. func isWordSep(c byte) bool { @@ -491,56 +1006,6 @@ func readHeredocDelim(script string, i int) (string, bool, int) { return b.String(), expand, i } -// applyKeyword updates the context stack for a standalone reserved word seen in -// command position. Compound-command openers push a counted frame; their -// closers pop the matching frame; continuation words (then/do/else/in) only -// restore command position for the word that follows. Anything else is an -// ordinary command name. -func applyKeyword(word string, stack *[]nestKind, cmdPos *bool, push func(nestKind) error, pop func()) error { - topIs := func(k nestKind) bool { return len(*stack) > 0 && (*stack)[len(*stack)-1] == k } - switch word { - case "if": - *cmdPos = true - return push(kindIf) - case "elif": - *cmdPos = true - return push(kindElif) - case "while", "until": - *cmdPos = true - return push(kindLoop) - case "for", "select": - *cmdPos = false // followed by a loop variable name, not a command - return push(kindLoop) - case "case": - *cmdPos = false // followed by the word being matched - return push(kindCase) - case "fi": - // Close an if ladder: pop any elif frames, then the matching if. - for topIs(kindElif) { - pop() - } - if topIs(kindIf) { - pop() - } - *cmdPos = false - case "done": - if topIs(kindLoop) { - pop() - } - *cmdPos = false - case "esac": - if topIs(kindCase) { - pop() - } - *cmdPos = false - case "then", "do", "else", "in": - *cmdPos = true - default: - *cmdPos = false // ordinary command name - } - return nil -} - // checkNestingDepth bounds the structural depth of a parsed AST. It is called // from [validateNode] — and therefore on every [Runner.Run] and [Runner.Check] // path — before the recursive [syntax.Walk] validation pass and before any diff --git a/interp/nesting_vuln_hunt_test.go b/interp/nesting_vuln_hunt_test.go index 767e05304..b77aca2a2 100644 --- a/interp/nesting_vuln_hunt_test.go +++ b/interp/nesting_vuln_hunt_test.go @@ -11,8 +11,11 @@ import ( "fmt" "os" "os/exec" + "runtime/debug" "strings" "testing" + + "mvdan.cc/sh/v3/syntax" ) // Regression coverage for the unbounded-nesting-depth stack-overflow crash @@ -169,6 +172,101 @@ func TestLexicalGuardTracksParserContexts(t *testing.T) { } } +// lexicalBypassScripts returns scripts whose real parser nesting is deep +// (depth levels) but which hide that nesting behind a construct that an +// earlier version of the lexical guard mis-tokenised — opening a phantom +// heredoc or quote, or leaving command position — so the guard undercounted. +// (Regressions for the second round of review findings on the guard.) +func lexicalBypassScripts(depth int) map[string]string { + r := strings.Repeat + parens := r("(", depth) + "x" + r(")", depth) + "\n" + subs := r("$(", depth) + ":" + r(")", depth) + "\n" + ifs := r("if x; then ", depth) + ":" + r("; fi", depth) + "\n" + return map[string]string{ + // "<<" is a shift inside arithmetic, not a heredoc operator. + "arith_shift": "echo $((1 << 2))\n" + parens, + "arith_shift_unspaced": "$((1<<9))\n" + parens, + "arith_shift_if": "echo $((1<<9))\n" + ifs, + "arith_command_shift": "((a<