Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions .github/THREAT_MODEL.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ Defaults are compatibility choices, not a hardened profile.
| `fetch` | Disabled, and never part of `WebApiFeatures.Default` | Only `UseFetch()` grants script outbound HTTP |
| Timeout, statement, memory, recursion limits | None | Untrusted execution is unbounded unless the host opts in |
| Parser source-length and AST-node limits | None | Hostile source and parser output are unbounded unless the host opts in |
| Stack overflow guard | Disabled | Native stack exhaustion can terminate the process |
| Stack overflow guard | Enabled | Interpreted function entry checks remaining native stack and turns exhaustion into a catchable `RangeError` |
| Maximum array size | `uint.MaxValue` | Effectively unbounded for hostile input |
| Regex timeout | 10 seconds | Bounds an individual regular expression operation |
| Promise wait timeout | 10 seconds | Bounds host APIs that wait for promise or module settlement |
Expand Down Expand Up @@ -342,17 +342,20 @@ quota.

**Missing or residual mitigation.**

- All stack protections are disabled by default.
- A host can explicitly disable `StackOverflowGuard`; doing so restores the native stack-exhaustion
process-termination risk.
- `LimitRecursion` counts repeated function definitions rather than every possible function
entry shape.
- `MaxExecutionStackCount` only covers call expressions and takes precedence over the more
complete stack overflow guard.
- The guard covers entry into interpreted functions, not arbitrary native stack consumption in
host callbacks or the CLR.
- A timeout alone is not a reliable stack-overflow defense.

**Required host action.** Enable `StackOverflowGuard` and configure a tested recursion
limit. Do not select the older `MaxExecutionStackCount` lane for untrusted code unless its
partial coverage is intentional. Keep process isolation so a runtime stack failure cannot
kill unrelated server workloads.
**Required host action.** Leave `StackOverflowGuard` enabled (or set it explicitly in a hardened
profile) and configure a tested recursion limit. Do not select the older
`MaxExecutionStackCount` lane for untrusted code unless its partial coverage is intentional.
Keep process isolation so a runtime or host stack failure cannot kill unrelated server workloads.

### TM-08: Blocking host calls and `Atomics.wait` evade timely cancellation

Expand Down Expand Up @@ -1203,6 +1206,7 @@ var engine = new Engine(options =>
options.Parsing.MaxSourceLength = 100_000;
options.Parsing.MaxNodeCount = 25_000;

// Enabled by default; keep it explicit in the hardened profile.
options.Constraints.StackOverflowGuard = true;
options.Constraints.PromiseTimeout = TimeSpan.FromSeconds(2);

Expand Down
30 changes: 18 additions & 12 deletions Jint.Tests.PublicInterface/HostStackOverflowGuardTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
namespace Jint.Tests.PublicInterface;

/// <summary>
/// What an embedder running untrusted script gets from <c>options.Constraints.StackOverflowGuard</c> when
/// that script recurses without bound: the promise a browser makes, a catchable <c>RangeError</c> and an
/// engine that is still usable afterwards, instead of a process that disappears.
/// What an embedder running untrusted script gets by default from
/// <c>options.Constraints.StackOverflowGuard</c> when that script recurses without bound: the promise a
/// browser makes, a catchable <c>RangeError</c> and an engine that is still usable afterwards, instead of
/// a process that disappears.
/// </summary>
/// <remarks>
/// <para>
/// The guard is opt-in, so every engine here but one asks for it. The exception is
/// <see cref="ADefaultEngineDoesNotProbeAtAll"/>, which pins the other half of the decision: a default
/// engine pays nothing and therefore protects nothing.
/// Every guarded engine here uses the default. <see cref="AnExplicitOptOutDoesNotProbeAtAll"/> pins that a
/// host which owns and independently bounds every script can still recover the probe's cost.
/// </para>
/// <para>
/// Every body runs on a dedicated thread with an explicit 1 MB stack, the size a thread-pool worker or an
Expand All @@ -24,7 +24,13 @@ public class HostStackOverflowGuardTests
{
private const int SmallStack = 1024 * 1024;

private static Engine Guarded() => new(options => options.Constraints.StackOverflowGuard = true);
private static Engine Guarded() => new();

[Fact]
public void TheGuardIsEnabledByDefault()
{
new Options().Constraints.StackOverflowGuard.Should().BeTrue();
}

public static TheoryData<string, string> UnboundedRecursions => new()
{
Expand Down Expand Up @@ -133,23 +139,23 @@ public void TheDepthFollowsTheStackTheEngineRunsOn()
}

/// <summary>
/// The default is off, and this is the only way to say so from a test: a default engine is run past
/// The explicit opt-out is pinned by running an unguarded engine past
/// the depth at which a guarded engine on the same 1 MB stack would have thrown, and must complete
/// the recursion normally instead. The run itself happens on a 16 MB stack, sixteen times what that
/// depth was measured against, because the assertion is about the absence of the guard's
/// <c>RangeError</c> and not about how much stack a default engine really has. Pinning the other
/// direction is impossible in-process: what a default engine does past its own limit is end the
/// <c>RangeError</c> and not about how much stack the unguarded engine really has. Pinning the other
/// direction is impossible in-process: what an unguarded engine does past its own limit is end the
/// process, and no test can assert on that.
/// </summary>
[Fact]
public void ADefaultEngineDoesNotProbeAtAll()
public void AnExplicitOptOutDoesNotProbeAtAll()
{
var guardedDepth = (int) MeasureGuardedDepth(SmallStack);

DedicatedThread.Run(
() =>
{
var engine = new Engine();
var engine = new Engine(options => options.Constraints.StackOverflowGuard = false);
engine.Execute("function f(n) { return n === 0 ? 0 : 1 + f(n - 1); }");

engine.Evaluate($"f({guardedDepth})").AsNumber().Should().Be(guardedDepth);
Expand Down
6 changes: 3 additions & 3 deletions Jint.Tests/Runtime/EngineLimitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public class EngineLimitTests
[Fact]
public void ShouldAllowReasonableCallStackDepth()
{
// A default engine has no stack guard (MaxExecutionStackCount is disabled), so this nesting
// depth runs directly against the native stack. The test runner's worker thread has an
// unpredictable amount of stack left, which made this test crash the process intermittently
// A default engine guards the native stack while MaxExecutionStackCount remains disabled. The
// test runner's worker thread has an unpredictable amount of stack left, which made this test
// crash the process intermittently
// (0xC00000FD) — run on a dedicated thread with an explicit stack instead. The explicit stack
// also makes the test platform-independent (the old macOS skip is no longer needed). The size
// is deliberately generous: StackOverflowException is uncatchable and would kill the whole test
Expand Down
15 changes: 5 additions & 10 deletions Jint.Tests/Runtime/StackOverflowGuardTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@
namespace Jint.Tests.Runtime;

/// <summary>
/// The opt-in backstop against unbounded script recursion
/// The default backstop against unbounded script recursion
/// (<see cref="Options.ConstraintOptions.StackOverflowGuard"/>). Without it every script below ends the
/// host process with a native stack overflow, which no <c>catch</c> can see and no test can assert on —
/// so the value of these tests is that they run at all.
/// </summary>
/// <remarks>
/// <para>
/// Every engine here is built with the guard on, because it is off by default: the benchmark gate priced
/// the probe at 1.7–2.3% on the recursion rows, above the 1% the decision rule allowed for shipping it on
/// by default. <c>Jint.Tests.PublicInterface.HostStackOverflowGuardTests</c> owns the other half of that
/// verdict, the pin that a default engine really does not probe.
/// Every engine here uses the default guard. The benchmark gate prices the probe honestly, while
/// <c>Jint.Tests.PublicInterface.HostStackOverflowGuardTests</c> pins both the default and the explicit
/// opt-out.
/// </para>
/// <para>
/// Every body runs on a dedicated thread with an explicit 1 MB stack: the platform default the guard has
Expand All @@ -29,11 +28,7 @@ public class StackOverflowGuardTests
{
private const int SmallStack = 1024 * 1024;

private static Engine Guarded(Action<Options>? configure = null) => new(options =>
{
options.Constraints.StackOverflowGuard = true;
configure?.Invoke(options);
});
private static Engine Guarded(Action<Options>? configure = null) => new(options => configure?.Invoke(options));

/// <summary>
/// One entry per route into a function body that does not go through a call expression. Every one of
Expand Down
6 changes: 3 additions & 3 deletions Jint/Native/Function/ScriptFunction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ protected internal override JsValue Call(JsValue thisObject, JsCallArguments arg
// CallExpression, and every one of those recursions ends as a native stack overflow when the
// only probe in the engine is the call expression's. Being first also keeps it outside the
// try/finally in CallOnce: a throw from inside it must not reach a finally that pops an
// execution context this frame never pushed. Off unless the host asked for it
// (Options.Constraints.StackOverflowGuard), in which case what is left here is one branch on a
// cached bool — see StackGuard.EnsureStackHeadroom.
// execution context this frame never pushed. On unless the host explicitly disables
// Options.Constraints.StackOverflowGuard or selects MaxExecutionStackCount; what is left here
// is one branch on a cached bool — see StackGuard.EnsureStackHeadroom.
//
// The probe belongs on the entry points that add a native frame, which is why it is here and in
// CallWithStackFrame rather than inside CallOnce: ContinueTailCalls re-enters CallOnce and
Expand Down
17 changes: 8 additions & 9 deletions Jint/Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ public class ConstraintOptions
/// <summary>
/// Whether every entry into an interpreted function probes the remaining native stack and throws a
/// catchable <c>RangeError: Maximum call stack size exceeded</c> when it is nearly gone. Defaults to
/// <see langword="false"/>.
/// <see langword="true"/>.
/// </summary>
/// <remarks>
/// <para>
Expand All @@ -851,13 +851,12 @@ public class ConstraintOptions
/// of tail position, and the non-call routes above.
/// </para>
/// <para>
/// It is off by default because it is not free. The probe runs at every entry into an interpreted
/// function, and the benchmark gate measured the recursion rows (<c>Fib</c>, <c>DeepSum</c>,
/// <c>Tak</c>) 1.7–2.3% slower with it on, agreeing across two order-rotated pairs, with hot shallow
/// calls unaffected. The rule fixed before that run was that shipping it on by default needed no
/// call row worse than about 1%, so it ships opt-in. A host whose scripts are all its own can bound
/// them with <see cref="MaxRecursionDepth"/> and pay nothing; a host sandboxing untrusted input
/// generally cannot, and a couple of percent on deep recursion is the price of staying alive.
/// It is enabled by default because the alternative on an unbounded recursion is termination of the
/// host process. The probe is not free: the benchmark gate measured the recursion rows (<c>Fib</c>,
/// <c>DeepSum</c>, <c>Tak</c>) roughly 1.5–3% slower with it on, while hot shallow calls stayed within
/// run-to-run noise. A host whose scripts are all trusted and independently bounded can explicitly
/// set this property to <see langword="false"/> to recover that cost. A host sandboxing untrusted
/// input generally cannot, and a couple of percent on deep recursion is the price of staying alive.
/// </para>
/// <para>
/// It is a backstop, not a policy. <see cref="MaxRecursionDepth"/> counts frames and is checked
Expand All @@ -876,7 +875,7 @@ public class ConstraintOptions
/// engine that already exists.
/// </para>
/// </remarks>
public bool StackOverflowGuard { get; set; }
public bool StackOverflowGuard { get; set; } = true;

/// <summary>
/// Maximum time a Regex is allowed to run, defaults to 10 seconds.
Expand Down
8 changes: 4 additions & 4 deletions Jint/Runtime/CallStack/StackGuard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ internal sealed class StackGuard
private readonly int _maxExecutionStackCount;
private readonly bool _enabled;

// Snapshot of Options.Constraints.StackOverflowGuard, which is opt-in and therefore false on a
// default engine. The two lanes are exclusive, and the reason is ordering rather than taste: the
// Snapshot of Options.Constraints.StackOverflowGuard, which is on for a default engine. The two
// lanes are exclusive, and the reason is ordering rather than taste: the
// backstop sits inside ScriptFunction, i.e. a few native frames *below* TryEnterOnCurrentStack's
// call site, so on a stack low enough for either to fire the backstop reaches the condition first
// and would throw where the older lane wanted to hop. MaxExecutionStackCount is the older and more
Expand All @@ -39,7 +39,7 @@ public StackGuard(Engine engine)
}

/// <summary>
/// The opt-in backstop against unbounded script recursion
/// The default backstop against unbounded script recursion
/// (<see cref="Options.ConstraintOptions.StackOverflowGuard"/>), run once per entry into an
/// interpreted function that adds a native frame — every route into one, not only a call
/// expression. Throws a catchable <c>RangeError</c> while there is still stack left to unwind on,
Expand All @@ -52,7 +52,7 @@ public StackGuard(Engine engine)
/// — <c>Call</c>, <c>CallWithStackFrame</c>, <c>CallFromRegisters</c> and <c>Construct</c> — pay one
/// predictable test of a <see langword="bool"/> field per call when the guard is off, which is what
/// makes it affordable to have the sites there unconditionally: the placement is what covers the
/// routes a call expression never sees, and it is worth keeping whether or not this engine opted in.
/// routes a call expression never sees, and it is worth keeping whether or not this engine opted out.
/// </para>
/// <para>
/// Those four are exactly the entries that grow the native stack, which is why the probe is not in
Expand Down
29 changes: 14 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2588,22 +2588,22 @@ for (var i = 0; i < 10; i++)

### Surviving an unbounded recursion

A script that recurses without bound does not throw. It exhausts the native stack of the thread it is
running on, and that ends the host process: no exception, nothing to `catch`, nothing in the log but an
exit code. Every route into a function body can do it — a call, `new`, a getter or setter, a
`valueOf`/`toString` coercion, a Proxy trap, a callback a built-in invokes, a host delegate that calls
back into the engine.
A script that recurses without bound can exhaust the native stack of the thread it is running on, and that
ends the host process: no exception, nothing to `catch`, nothing in the log but an exit code. Every route
into a function body can do it — a call, `new`, a getter or setter, a `valueOf`/`toString` coercion, a Proxy
trap, a callback a built-in invokes, a host delegate that calls back into the engine.

```c#
var engine = new Engine(options =>
{
options.Constraints.StackOverflowGuard = true;
// Only opt out when every script is trusted and independently bounded.
options.Constraints.StackOverflowGuard = false;
});
```

With the guard on, every entry into a script function first checks that the native stack has not been used
up, and the same script raises `RangeError: Maximum call stack size exceeded` while there is still room to
unwind — an ordinary JavaScript error, catchable by the script itself and by your
The guard is on by default. Every entry into a script function first checks that the native stack has not
been used up, and an unbounded recursion raises `RangeError: Maximum call stack size exceeded` while there
is still room to unwind — an ordinary JavaScript error, catchable by the script itself and by your
`catch (JavaScriptException)`, with the engine still usable afterwards. It measures the remaining stack
rather than counting calls, so it covers all of those routes and adapts to the thread the engine runs on:
a host that provisions a larger stack gets proportionally more depth, rather than the frame count someone
Expand All @@ -2615,11 +2615,10 @@ check sits on the entries that do add a frame, so a strict tail recursion neithe
stopped by it. Sloppy-mode recursion, a call out of tail position, and the non-call routes above are what
remain in scope.

It is off by default because the check runs at every entry into a script function and that is not free.
The benchmark gate measured recursion-heavy workloads (`Fib`, `DeepSum`, `Tak`) 1.7–2.3% slower with it
on, with hot shallow calls unaffected. Enable it when the engine runs script you did not write, where a
couple of percent on deep recursion in exchange for keeping your process is plainly the trade you want.
Leave it off when every script is yours and you can bound it yourself.
The check runs at every entry into a script function and is not free. The benchmark gate measured
recursion-heavy workloads (`Fib`, `DeepSum`, `Tak`) roughly 1.5–3% slower with it on, while hot shallow
calls stayed within run-to-run noise. Jint accepts that default cost because terminating the host process
is worse. Set `StackOverflowGuard` to `false` only when every script is trusted and independently bounded.

`options.LimitRecursion(n)` answers a different question and composes with it. The recursion limit counts
frames and is checked before the callee is entered, so where it is configured it is what fires; the guard
Expand Down Expand Up @@ -3116,7 +3115,7 @@ a hardened deployment baseline.
- Limit depth of calls to prevent deep recursion calls.
- Define a timeout, to prevent scripts from taking too long to finish.
- Turn an unbounded recursion into a catchable error rather than a stack overflow that ends the process
(see [Surviving an unbounded recursion](#surviving-an-unbounded-recursion); off by default).
(see [Surviving an unbounded recursion](#surviving-an-unbounded-recursion); enabled by default).

Parsing happens before execution constraints start, so hosts accepting untrusted source should bound it
separately:
Expand Down