1033253 - "Assertion failure: uintptr_t(str) > 0x1000" with a specific WebRTC constraints object due to function Xrays not handling .name on anonymous functions properly

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: uintptr_t(str) > 0x1000, at ../../../dist/include/js/Value.h:721

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jesse Ruderman]

This testcase doesn't crash, but I don't trust anything that gets near jsval_layout.asBits.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Very likely (to be confirmed) this is a trying to set a nullptr into a JS string; it's possible it's an uninitialized value.  Once we take a look at the case it should become clear which one is happening. 

Jesse: I'm guessing if it's a guaranteed nullptr, that's likely not a sec issue.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: Throw if mandatory.constraint is not an object

I'm a bit surprised that the webidl bindings let a function through as a webidl object argument, but here you go.

This part of the code needs updating anyways to match advancements in the spec (CreateOffer/Answer no longer takes constraints but a dictionary). I'll open a separate bug.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> I'm a bit surprised that the webidl bindings let a function through as a webidl object
> argument

Web IDL "object" means any JS object.  Functions are objects in JS.

I don't care that much whether you want to filter them out here or not (in a separate bug, please), though per spec I assume you shouldn't.  But the real bug here is in the Xray code, which for a function Xray does this:


  } else if (id == GetRTIdByIndex(cx, XPCJSRuntime::IDX_NAME)) {
     FillPropertyDescriptor(desc, wrapper, JSPROP_PERMANENT | JSPROP_READONLY,
                            StringValue(JS_GetFunctionId(JS_GetObjectFunction(target))));

But for an anonymous function JS_GetFunctionId will return null, which is what's triggering the assert.  WebRTC is just a way of triggering this codepath.  Here are simpler STR:

1)  Load data:text/html,<script>document.documentElement.onclick = function() {}</script>
2)  In a browser-environment scratchpad, evaluate:

       content.document.documentElement.onclick.name

This code needs to null-check the return value of JS_GetFunctionId.

Comment 6

[Bobby Holley (:bholley)]

Attachment: Null-check the result of JS_GetFunctionId. v1

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8449574 [details] [diff] [review]
Null-check the result of JS_GetFunctionId. v1

r=me

Comment 8

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ffddc07896d7

Comment 9

[Andrew McCreight [:mccr8]]

Sounds like a null-deref.  I'm going to leave it hidden for now because I'm not sure if any of the other implications of the test case are worth hiding or not.

Comment 10

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8449511 [details] [diff] [review]
Throw if mandatory.constraint is not an object

(In reply to Boris Zbarsky [:bz] from comment #5)
> I don't care that much whether you want to filter them out here or not (in a
> separate bug, please), though per spec I assume you shouldn't.

Thanks for spotting the real bug. I've filed Bug 1033833 to use dictionaries instead of constraints here, so that takes care of the filtering question.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ffddc07896d7

Comment 12

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed assert in Fx33, 2014-07-01.
Verified fixed in Fx33, 2014-10-06.