Closed Bug 1577573 Opened 2 months ago Closed 2 months ago

webExtension content-script can cause a crash in the content process using exportFunction

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: baku, Assigned: bzbarsky)

Details

(Keywords: sec-low, Whiteboard: [adv-main70+][adv-main70-rollup])

Attachments

(2 files)

This doesn't seem to be a bug in webExtension code. More likely it's in the implementation of 'exportFunction'. Attach to this bug there is a small test case. Load it from about:debugging -> Load temporary extension... -> load any website.

The content script does:

function overrideProp(object, property, original) {
  let newValue = new window.wrappedJSObject.Proxy(original, {
      apply: exportFunction(function(target, thisArg, args) {
        if (ContentScript.syncShouldOverload()) {
          if (data.potentiallyShowContextBanner) {
            ContentScript.syncPotentiallyShowContextBanner();
          }
          return window.wrappedJSObject.Promise.reject(new window.wrappedJSObject.Error("SecurityError"));
        }
        return window.wrappedJSObject.Reflect.apply(target, thisArg, args);
      }, window),
    });

  object[property] = exportFunction(newValue, window);
}

overrideProp(window.navigator.wrappedJSObject, "mozGetUserMedia", window.navigator.wrappedJSObject.mozGetUserMedia);

I'll provide a valid crash report soon.

#0 0x00007fb4776bb073 in FunctionFlags::hasFlags(unsigned short) const (this=0x22, flags=256) at /home/baku/Sources/m/proxy/src/js/src/vm/JSFunction.h:147
#1 0x00007fb4776e22ea in FunctionFlags::hasInferredName() const (this=0x22) at /home/baku/Sources/m/proxy/src/js/src/vm/JSFunction.h:187
#2 0x00007fb4776e2289 in JSFunction::hasInferredName() const (this=0x0) at /home/baku/Sources/m/proxy/src/js/src/vm/JSFunction.h:455
#3 0x00007fb4776b7469 in JSFunction::explicitName() const (this=0x0) at /home/baku/Sources/m/proxy/src/js/src/vm/JSFunction.h:560
#4 0x00007fb477e92a35 in JS_GetFunctionId(JSFunction*) (fun=0x0) at /home/baku/Sources/m/proxy/src/js/src/jsapi.cpp:3440
#5 0x00007fb46ff793bf in xpc::ExportFunction(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
(cx=0x563b48e0c790, vfunction=..., vscope=..., voptions=..., rval=...) at /home/baku/Sources/m/proxy/src/js/xpconnect/src/ExportHelpers.cpp:430
#6 0x00007fb46ff7fc74 in SandboxExportFunction(JSContext*, unsigned int, JS::Value*) (cx=0x563b48e0c790, argc=2, vp=0x563b49d519c0) at /home/baku/Sources/m/proxy/src/js/xpconnect/src/Sandbox.cpp:374
#7 0x00007fb4776b437c in CallJSNative(JSContext*, bool ()(JSContext, unsigned int, JS::Value*), JS::CallArgs const&)

Looks like either an xpconnect or js issue, redirecting to xpconnect for now.

Component: General → XPConnect

So looking at xpc::ExportFunction, we get a HandleValue vfunction. We testd that it's an object, then do:

    RootedObject funObj(cx, &vfunction.toObject());
...
    funObj = js::CheckedUnwrapStatic(funObj);
    // Check that funObj is not null
    funObj = UncheckedUnwrap(funObj);
    if (!JS::IsCallable(funObj)) {
      JS_ReportErrorASCII(cx, "First argument must be a function");
      return false;
    }
...
    JSFunction* fun = JS_GetObjectFunction(funObj);
    RootedString funName(cx, JS_GetFunctionId(fun));

so that looks like the buggy part: in the testcase here funObj is a callable Proxy. So it tests true for JS::IsCallable, but it's NOT a JSFunction, so the JS_GetObjectFunction call return null, and then JS_GetFunctionId(fun) leads to a null-deref; note the fun=0x0 in frame 4 of the stack in comment 2.

I think we should open this up; it's a null-deref and one that requires somewhat privileged code (at least an extension) to trigger.

Group: dom-core-security
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8df1e9b9f393
Make sure we actually have a function in exportFunction before we try to get its name.  r=mccr8
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Assignee: nobody → bzbarsky
Whiteboard: [adv-main70+][adv-main70-rollup]
You need to log in before you can comment on or make changes to this bug.