Potential assertion failure in MessagePort structured cloning

RESOLVED FIXED in mozilla33

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jmorton, Assigned: jmorton)

Tracking

Trunk
mozilla33
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
See http://mxr.mozilla.org/mozilla-central/source/dom/base/MessagePort.cpp#188

This callback should not return true and a null returnObject. If JS_WrapObject fails, this could happen.
(Assignee)

Updated

4 years ago
Assignee: nobody → jmorton
(Assignee)

Comment 1

4 years ago
Created attachment 8450687 [details] [diff] [review]
0001-Bug-1034380-Prevent-MessagePort-transferable-callbac.patch
Attachment #8450687 - Flags: review?(bzbarsky)
Comment on attachment 8450687 [details] [diff] [review]
0001-Bug-1034380-Prevent-MessagePort-transferable-callbac.patch

This doesn't handle the case when "obj" is null.  It also relies on falling through out of our tag == SCTAG_DOM_MAP_MESSAGEPORT block, which is ok for now, but would get worse once we had more cases.

I'd prefer we just do early returns of false as needed:

  if (!obj) {
    retunr false;
  }

etc.
Attachment #8450687 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 3

4 years ago
Created attachment 8451769 [details] [diff] [review]
0001-Bug-1034380-Prevent-MessagePort-transferable-callbac.patch

Hopefully more like what you were thinking.
Attachment #8450687 - Attachment is obsolete: true
Attachment #8451769 - Flags: review?(bzbarsky)
Comment on attachment 8451769 [details] [diff] [review]
0001-Bug-1034380-Prevent-MessagePort-transferable-callbac.patch

Looks great, thanks.  r=me.
Attachment #8451769 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

4 years ago
Created attachment 8452522 [details] [diff] [review]
0001-Bug-1034380-Prevent-MessagePort-transferable-callbac.patch

r=bz
Attachment #8451769 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
hi,

could you provide a Try link. Suggestions for what to run if you haven't
yet can be found here:
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/61a09b7b7e9d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.