Closed Bug 565604 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5
Tracking Status
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: jruderman, Assigned: Waldo)

Details

(Keywords: assertion, testcase, Whiteboard: [sg:critical?][critsmash:investigating])

Attachments

(1 file)

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
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.
Attached patch Patch and test (deleted) — Splinter Review
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.
http://hg.mozilla.org/mozilla-central/rev/f9eb2652b13d 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]
Status: NEW → RESOLVED
Closed: 15 years ago
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: