ReviewOS

zig-utils/zig-js

Parser: indirect eval leaks the caller's new.target permission into global code

#900
Closed chris opened this 11 days ago ·
11 days ago

Parent: #459

Problem

This is a missing early error, not a message-text divergence — found while sweeping error messages, but it is a spec-conformance bug.

new.target outside a function or class static block is an early SyntaxError (ECMA-262 sec-static-semantics-early-errors for NewTarget). zig-js accepts it and evaluates it, so the whole program runs where it should have been rejected at parse time:

eval("new.target")                    // JSC: SyntaxError   zig-js: no throw
eval("var t = new.target; t")         // JSC: SyntaxError   zig-js: no throw
eval("if (true) { new.target; }")     // JSC: SyntaxError   zig-js: no throw

JavaScriptCore: SyntaxError: new.target is only valid inside functions or static blocks.

Verified directly against a real JSC backend, and reproduced independently of the sweep agent that first surfaced it.

The legal positions already behave correctly, so only the rejection is missing:

(function () { return new.target; })()                    // fine on both
class C { m() { return new.target; } } new C().m()        // fine on both

Why it matters beyond wording

Every other item in this sweep is a string difference. This one changes which programs run: a typo'd new.target at top level silently evaluates to undefined instead of failing at parse time, so the error surfaces far from its cause.

Note on the existing gate

parseNew already has the machinery — it checks self.new_target_depth == 0 and returns ParseError.UnexpectedToken for the meta-property. Worth checking why the top-level case does not reach that gate (script/eval top level may be seeding a nonzero depth), since the fix may be to the depth bookkeeping rather than to the check.

test262 has coverage under test/language/expressions/new.target/, which currently passes at 12/12 — so whatever it asserts does not include the top-level rejection, and a case should be added alongside the fix.

11 days ago

Correcting this issue: my original description was wrong. I filed it from a sweep agent's report without reproducing it myself first, and over-generalized.

Top-level new.target is correctly rejected. So is a direct eval at true top level, and so is an indirect eval at top level. Measured:

shapezig-jsJavaScriptCore
new.target in global codeSyntaxError ✓SyntaxError
if (true) { new.target; } in global codeSyntaxError ✓SyntaxError
var z = new.target in global codeSyntaxError ✓SyntaxError
direct eval("new.target") at true top levelSyntaxError ✓SyntaxError
indirect eval at top level, then direct eval insideSyntaxError ✓SyntaxError
inside a function → indirect eval → direct evalno throw ✗SyntaxError
inside a function → direct evalno throw ✓no throw ✓

The last two rows are the point. A direct eval does inherit its caller's new.target, so a direct eval inside a function correctly accepts it. The bug is that an indirect eval invoked from inside a function does not clear that permission, so the global code it creates keeps it, and a direct eval nested in that global code wrongly accepts new.target.

function f() { return (0, eval)("try { eval('new.target'); 'no throw' } catch (e) { e.name }"); }
f()   // zig-js: "no throw"   JSC: "SyntaxError"

Root cause

Interpreter.direct_eval_new_target_allowed is set true whenever a non-arrow function activation is entered, and evalFn seeds parser.new_target_depth = 1 from it. Nothing cleared it when an indirect eval switched to global code, so the flag survived the function → global-code boundary.

The parser's gate (new_target_depth == 0 in parseNew) was correct all along; it was being handed a non-zero depth.

Scope correction

This is one leaked flag across the indirect-eval boundary, not a missing early error. The title should read something like "indirect eval leaks the caller's new.target permission into global code".

The 12/12 pass on test/language/expressions/new.target/ is therefore consistent — the corpus does not cover a direct eval nested inside indirect-eval'd global code.

11 days ago

Fixed in 7b92b32.

Interpreter.evalFn now clears direct_eval_new_target_allowed for the duration of an indirect eval, so the permission cannot outlive the function that granted it. The parser gate needed no change — it was being handed a nonzero new_target_depth.

Behaviour pinned by a new test in src/context.zig, covering both directions:

shapebeforeafter
inside a function → indirect eval → direct evalno throwSyntaxError
new.target in global codeSyntaxErrorSyntaxError
direct eval at true top levelSyntaxErrorSyntaxError
indirect eval at top level → direct evalSyntaxErrorSyntaxError
direct eval inside a functionacceptedaccepted
arrow inheriting an enclosing function's bindingacceptedaccepted
granting function's own binding after it performs an indirect evalacceptedaccepted

--diag: 5,071 pass / 0 fail across expressions/new.target 14, eval-code 347, statements/class 4,367 (covers static blocks) and expressions/arrow-function 343.

One deliberate non-match

class C { static { new.target } } is legal per spec and zig-js evaluates it. JSC throws ReferenceError: Can't find private variable: PrivateSymbol.newTargetLocal, leaking an internal symbol name — a JSC bug. Not matching that one.

Note on the arrow case

var A = () => eval('new.target'); function H(){ return A(); } new H() correctly throws: the arrow was created in global code, so it has no binding to inherit regardless of who calls it. zig-js and JSC agree on the behaviour; only the message text differs, which is #899.

Sign in to comment on this issue.