Bugzilla@Mozilla – Bug 494445
MirrorWrappedNativeParent result sometimes uninitialized [@ js_LockGCThingRT ]
Last modified: 2009-07-21 17:15:17 PDT
Summon comment box
Created attachment 379193 [details] [review] v1 I was looking at crash reports, there's a number of crashes in js_LockGCThingRT being called from XPCNativeWrapper::GetNewOrUsed. We call js_LockGCThingRT on the result of a call to MirrorWrappedNativeParent. Turns out that MirrorWrappedNativeParent sometimes returns PR_TRUE without initializing the result out pointer. I think setting it to null would be the right thing to do. I think that this probably shows up under a number of different crash signatures, because XPCNativeWrapperCtor also calls MirrorWrappedNativeParent and it uses the uninitialized pointer to set it as a parent by calling JS_SetParent. Nominating for blocking and marking security sensitive.
Talked this over with peterv a bit. It's a nasty crash that results in an uninitialized pointer in the JS heap. Since we have a pretty simple patch, and it's a security issue, I say we block and get this in asap. If it comes down the the wire (*cough* may be there already) we can move on without this, but it doesn't seem to make sense to hold this back.
Comment on attachment 379193 [details] [review] v1 I spent a while tracking down why we do this mirroring at all (it isn't clear what we're protecting with it) and the result I came up with was that this patch makes things no worse than they are now wrt XPCNativeWrappers' parents.
Created attachment 379226 [details] [review] v1.1 With NS_OUTPARAM for static analysis goodness. Carrying forward r/sr=mrbkap.
BTW, this is not a problem on older branches (but would be if bug 455633 is landed there).
setting wanted1.9.0.x? flag just so we don't keep triaging it when we go through trunk-fixed security bugs we haven't evaluated for the 1.9.0 branch. Better than a minus, which is true at the moment but might cause us to ignore this bug if bug 455633 is ever backported.
We are landing bug 455633 on the 1.9.0 branch to fix regression bug 502458, therefore we need this fix too (rolled into the branch patch in bug 455633).
mrbkap landed this fix with the fix for bug 455633 on the 1.9.0 branch.