Skip to content

fix(babel-traverse): deopt path.evaluate() on native call errors#18003

Open
raghavwahi wants to merge 4 commits into
babel:mainfrom
raghavwahi:fix/17644-evaluate-stack-overflow
Open

fix(babel-traverse): deopt path.evaluate() on native call errors#18003
raghavwahi wants to merge 4 commits into
babel:mainfrom
raghavwahi:fix/17644-evaluate-stack-overflow

Conversation

@raghavwahi
Copy link
Copy Markdown

@raghavwahi raghavwahi commented May 17, 2026

Q A
Fixed Issues? Fixes #17644
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? No
License MIT

Summary

path.evaluate() crashes with RangeError: Maximum call stack size exceeded on input like:

String({ toString: "".toUpperCase })

The CallExpression branch of the evaluator invokes the real native function (here, String(obj)) via func.apply(context, args). If the evaluator has previously produced an object whose own coercion methods recurse infinitely (the ObjectExpression branch allows function-valued properties, so obj.toString === String.prototype.toUpperCase is reachable), that native call blows the stack instead of being caught.

This wraps the native call in try/catch and deopts on any thrown error, which matches how path.evaluate() already gives up in other uncertain cases (e.g., inherited methods — see existing should not evaluate inherited methods test).

Change

try {
  return func.apply(context, args);
} catch {
  deopt(path, state);
  return;
}

After the fix, path.evaluate() on the issue's input returns { confident: false, value: undefined, deopt: <path> } — no crash.

Tests

Added a regression test in packages/babel-traverse/test/evaluation.js using the issue's exact input. Full yarn jest packages/babel-traverse suite is green (652 passed, 1 skipped), make lint clean.

Note for reviewers - related, out-of-scope crash

If a caller stores the result of path.evaluate().value for an ObjectExpression like { toString: "".toUpperCase } and later coerces that returned object to a string in their own code, it will still crash with the same RangeError — because the returned value is a real JS object with a malicious toString. This PR intentionally does not address that path, because:

  • The issue as filed only asks about the crash inside path.evaluate() itself, which is now fixed.
  • Avoiding it would require either (a) refusing to build objects whose values are function references, or (b) returning a wrapped/safe value - both of which are larger design decisions than the reported bug calls for.

Happy to follow up in a separate PR / issue if maintainers want broader hardening of the ObjectExpression branch.

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented May 17, 2026

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61623

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 17, 2026

Open in StackBlitz

commit: 5ac7d21

@raghavwahi raghavwahi marked this pull request as draft May 18, 2026 17:47
@raghavwahi raghavwahi marked this pull request as ready for review May 18, 2026 17:47
@nicolo-ribaudo
Copy link
Copy Markdown
Member

I would prefer if we deopted every time we pass an object to String(), would that be possible? RangeErrors are not always catchable.

@liuxingbaoyu
Copy link
Copy Markdown
Member

I have a deopt fix on my local machine, and I will commit it to this branch.

@liuxingbaoyu
Copy link
Copy Markdown
Member

We have to ban toString, what do you think? @nicolo-ribaudo

@liuxingbaoyu
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ac7d21b8e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +412 to +414
typeof right === "object" &&
Object.hasOwn(right, "toString")
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard null before Object.hasOwn in + deopt check

This branch treats typeof right === "object" as safe, but null also has type "object"; calling Object.hasOwn(null, "toString") throws TypeError and crashes path.evaluate() for inputs like "" + null instead of returning a confident value. This is a regression introduced by the new deopt guard and should explicitly exclude null before calling Object.hasOwn.

Useful? React with 👍 / 👎.

Comment on lines +437 to +439
(typeof left === "object" &&
typeof right === "string" &&
Object.hasOwn(left, "toString")) ||
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard null before Object.hasOwn in == deopt check

Both sides of this == guard rely on typeof x === "object", which includes null; expressions such as "" == null or null == "" now throw TypeError from Object.hasOwn instead of evaluating normally. Since this path runs inside evaluate(), the new check introduces a crash for previously supported inputs.

Useful? React with 👍 / 👎.

Comment on lines +520 to +523
func === String &&
args.length > 0 &&
Object.hasOwn(args[0], "toString")
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle null/undefined in String(...) deopt guard

The new String special-case calls Object.hasOwn(args[0], "toString") without checking for null/undefined, so common calls like String(undefined) and String(null) now throw TypeError during evaluation. That changes behavior from returning a confident constant to crashing, which is an urgent regression in constant folding.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Running path.evaluate() on "maliciously crafted code" causes a crash

4 participants