"Assertion failure: getClass()->flags & JSCLASS_HAS_PRIVATE" after setting __proto__ of a WebGLIntArray

RESOLVED FIXED in mozilla1.9.3a5



JavaScript Engine
8 years ago
7 years ago


(Reporter: Jesse Ruderman, Assigned: Waldo)


(Blocks: 1 bug, {assertion, testcase})

assertion, testcase

Firefox Tracking Flags

(status1.9.2 unaffected, status1.9.1 unaffected)


(Whiteboard: [sg:critical?][critsmash:investigating])


(1 attachment)



8 years ago
I initially hit this in the browser, but these testcases work in xpcshell.

Testcase 1:
o = {}; o.__proto__ = WebGLIntArray(1); o.length

Testcase 2:
o = Components.interfaces.nsIAtom; o.__proto__ = WebGLIntArray(1); o.length

Testcase 1 reliably produces
Assertion failure: getClass()->flags & JSCLASS_HAS_PRIVATE, at js/src/jsobj.h:391

Testcase 2 gives strange numbers (indicating a type safety violation) or
Assertion failure: INT_FITS_IN_JSVAL(i), at js/src/jsapi.h:252
Assignee: general → jwalden+bmo


8 years ago
Whiteboard: [sg:critical?][critsmash:investigating]
Waldo, any luck here?
Just a simple failure to class-check inside a property getter, due to the usual problem of a class-requiring getter being invoked on a different object through the wonders of the prototype chain.  Easy enough to fix, should have a patch tomorrow.
Created attachment 446311 [details] [diff] [review]
Patch and test

The first instance is with a class without a private, and getPrivate() asserts privacy.  The second instance is with a class *with* a private, and we cast it to something bogus and get something bogus back.
Attachment #446311 - Flags: review?(vladimir)
Comment on attachment 446311 [details] [diff] [review]
Patch and test

Looks fine to me, though I wrote the bug in the first place so maybe i'm not the best judge :-)  Do any changes need to happen on the tracing side?

Also you'll have to do a bit of merging in the CustomQS_WebGL side of things -- two of those functions (BufferData and TexImage2D) can actually accept a null data argument; I just landed a patch that enables/fixes that.
Attachment #446311 - Flags: review?(vladimir) → review+
I don't believe any tracing changes are necessary.  This is basically applying the treatment in array_length_getter to typed arrays, and I'm aware of no issues with that.

The good side of this bug, so far as it exists, is that none of the affected properties do anything but read memory.  Thus the worst that can happen is a crash or a read from an arbitrarily choosable memory location (arbitrary because there's no private slot in objects without privates, as Shaver so astutely reminds me, so the "private slot" value is user-controllable) -- information and memory disclosure, but no code execution or anything worse.

I made the necessary tweaks but ran out of time to push today, will do tomorrow.
Whiteboard: [sg:critical?][critsmash:investigating] → [sg:moderate]
note that we never shipped this code anywhere except maybe a trunk alpha.

Actually, I was wrong about this only being sg:moderate -- I forgot that one of the accessors is .buffer which returns an object whose contents are then arbitrarily choosable.  I discovered this when I added in tests for the other properties (since .length goes only through the very specialized JSOP_LENGTH path) and discovered a truly egregious typo in the implementation of .byteOffset which pointed out the object-valued property -- fixed in the landing.

I tend to hold trunk-only bugs closed for a little while after fix (to get most nightly users upgraded) plus the next alpha and a week-ish (to catch beta-channeling users).  Past that it's of course better to open it up.  This has been in since the original typed-array landing, so it's certainly in at least one or two alphas and thus needs some delay for straggling beta users to upgrade.
Whiteboard: [sg:moderate] → [sg:critical?][critsmash:investigating]
Last Resolved: 8 years ago
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Group: core-security
status1.9.1: --- → unaffected
status1.9.2: --- → unaffected
You need to log in before you can comment on or make changes to this bug.