Closed Bug 655098 Opened 9 years ago Closed 8 years ago

CreateNPObjectMember jsids don't appear to be rooted

Categories

(Core :: Plug-ins, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla6
Tracking Status
firefox5 - wontfix
firefox6 + fixed
firefox7 + fixed
blocking1.9.2 --- .23+
status1.9.2 --- .23-fixed

People

(Reporter: benjamin, Assigned: benjamin)

Details

(Whiteboard: [sg:critical?][qa-ntd-192][qa-])

Attachments

(2 files)

The jsid in CreateNPObjectMember may be a non-rooted identifier. http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsJSNPRuntime.cpp#2089

It should be traced at http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsJSNPRuntime.cpp#2292

But currently the JSAPI doesn't actually expose a public function for tracing jsid.

Filing as private becuase it's theoretically possible to abuse this via script to perhaps dereference string characters which were GCed.
(In reply to comment #0)
> But currently the JSAPI doesn't actually expose a public function for tracing
> jsid.

We should add one, but in the meantime you can do if (JSID_IS_STRING(id)) JS_CALL_STRING_TRACER(trc, JSID_TO_STRING(id), "some_name")
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #530636 - Flags: review?(mrbkap)
Attachment #530636 - Flags: review?(cdleary)
Attachment #530636 - Flags: review?(mrbkap) → review+
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

Review of attachment 530636 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry I didn't see this earlier. Got buried in my email.
Attachment #530636 - Flags: review?(cdleary) → review+
summary sounds bad, guessing at severity rating. Is this in old code (affecting 1.9.2.x as well)?
status1.9.2: --- → ?
Whiteboard: [sg:critical?]
Yes, this affects all older branches. I guess it's possible to exploit this... not easy, though.
Can we get this patch landed?
http://hg.mozilla.org/mozilla-central/rev/49e57fa259dc
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Attachment #530636 - Flags: approval-mozilla-aurora?
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

Approved for mozilla-aurora
Attachment #530636 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
blocking1.9.2: ? → .19+
bsmedberg, can we get this landed on aurora?
Do we need a different patch for the 1.9.2 branch?
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

This patch will probably apply to 1.9.2 manually using the old location (modules/plugin/base/src) instead of dom/plugins/base.
Attachment #530636 - Flags: checkin?
Attachment #530636 - Flags: checkin?
Somehow this got marked a branch blocker without me noticing. This might be the correct branch version of this, but I'm not sure and I'm not sure it's really serious enough to block.
Attachment #549927 - Flags: review?(cdleary)
Attachment #549927 - Flags: review?(cdleary) → review+
Comment on attachment 549927 [details] [diff] [review]
1.9.2 branch version, maybe, rev. 1

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/48b6fb82a960
Attachment #549927 - Flags: approval1.9.2.20+
Are there any STR for this issue to use for verification of this fix?
blocking1.9.2: .20+ → .21+
Whiteboard: [sg:critical?] → [sg:critical?][qa-examined-192][qa-needs-STR]
No, it was entirely theoretical.
Ok. Marking this as "Nothing to Do" for QA.
Whiteboard: [sg:critical?][qa-examined-192][qa-needs-STR] → [sg:critical?][qa-ntd-192]
qa-: fix needs no QA verification
Whiteboard: [sg:critical?][qa-ntd-192] → [sg:critical?][qa-ntd-192][qa-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.