Closed Bug 1782865 Opened 2 years ago Closed 2 years ago

v8::internal::Isolate::trace is not called

Categories

(Core :: JavaScript Engine, task)

task

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox105 --- fixed

People

(Reporter: arai, Assigned: arai)

Details

Attachments

(1 file)

v8::internal::Isolate has trace method and it traces values in handleArena_,
but apparently trace is not called anywhere.

https://searchfox.org/mozilla-central/rev/c454f13d83dc85f399fd1eb449ea9ccb156299df/js/src/irregexp/RegExpShim.h#1086

class Isolate {
...
  void trace(JSTracer* trc);

https://searchfox.org/mozilla-central/rev/c454f13d83dc85f399fd1eb449ea9ccb156299df/js/src/irregexp/RegExpShim.cpp#134-141

void Isolate::trace(JSTracer* trc) {
  js::gc::AssertRootMarkingPhase(trc);

  for (auto iter = handleArena_.Iter(); !iter.Done(); iter.Next()) {
    auto& elem = iter.Get();
    JS::GCPolicy<JS::Value>::trace(trc, &elem, "Isolate handle arena");
  }
}

I assume it's supposed to be called in JSContext::trace, after it gets stored into JSContext::isolate field,
but so far I cannot find any case that handleArena_ becomes non-empty at the point of GC/tracing.
so this method might be just unnecessary, but marking as security-sensitive just in case.

Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Group: core-security → javascript-core-security

Setting as sec-low, given currently there's no possible path that can hit the issue,
and the trace method is no-op.

This patch will prevent issue in possible future change that triggers GC inside regexp.

Keywords: sec-low

No need to hide this if it is just future proofing. Thanks for fixing this.

Group: javascript-core-security
Keywords: sec-low
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: