XPCConvert::NativeInterface2JSObject should create COWs automatically

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: smaug, Assigned: mrbkap)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+, blocking1.9.2 -, status1.9.2 wontfix, blocking1.9.1 -, status1.9.1 wontfix)

Details

(Whiteboard: [sg:critical][compartments][hardblocker])

Attachments

(1 obsolete attachment)

Updated

8 years ago
Whiteboard: [sg:critical]
mrbkap, can you have a look at his one? We're interested in this for branches ASAP.
Assignee: nobody → mrbkap

Updated

8 years ago
Whiteboard: [sg:critical] → [sg:critical][critsmash:investigating]

Comment 2

8 years ago
blake, can we get an ETA on this?
As a note, bug 523994 fixes this in a really clean way (and makes NativeInterface2JSObject actually legible too). Unfortunately, I'm bogged down in fixing-mochitest hell in that bug, so I'll fix this one separately so we can get it landed while I finish bug 523994.
Created attachment 457749 [details] [diff] [review]
Proposed fix

Of special importance in reviewing this patch is the control flow in NI2JSO: I looked carefully and as far as I can tell, it is safe to have an early return here. Also, how we determine that we are returning to content is a little skeezy (using filename flags is pretty terrible). Also, figuring out that the object is chrome is not great. bug 523994 will again come to rescue us.
Attachment #457749 - Flags: review?(jst)

Updated

8 years ago
Attachment #457749 - Flags: review?(jst) → review+

Updated

8 years ago
Whiteboard: [sg:critical][critsmash:investigating] → [sg:critical][critsmash:patch]
Are we done here?  Can we land?
Test?

Comment 7

8 years ago
blake?
Sorry, the patch causes a ton of assertions on mochichrome tests. I'll attach a new one tomorrow.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
status1.9.1: --- → wanted
status1.9.2: --- → wanted
status2.0: --- → wanted

Updated

8 years ago
blocking2.0: --- → betaN+
status2.0: wanted → ---

Updated

8 years ago
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
Where's that new patch, mrbkap?

Updated

8 years ago
Attachment #457749 - Attachment is obsolete: true
Blake, seriously, is there a new patch coming?
Whiteboard: [sg:critical][critsmash:patch] → [sg:critical][critsmash:patch][compartments]
(we'll need to do something special for the older branches)
As soon as compartments lands, we should verify this is fixed.  Marcia?
Blake: Is there any an easy way I can verify that this bug is fixed? I don't see a testcase readily available.
Marcia, the testcase in bug 572129 might show whether this got fixed by the compartments landing or not.
(Reporter)

Comment 15

8 years ago
Marcia, did you manage to verify whether this is fixed or not?
When I run the test case from the bug in Comment 14 using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101116 Firefox/4.0b8pre, I still get the 
alert dialog that shows "Components.stack."

Updated

8 years ago
Whiteboard: [sg:critical][critsmash:patch][compartments] → [sg:critical][critsmash:patch][compartments]hardblocker

Updated

8 years ago
Whiteboard: [sg:critical][critsmash:patch][compartments]hardblocker → [sg:critical][compartments]hardblocker

Updated

8 years ago
Whiteboard: [sg:critical][compartments]hardblocker → [sg:critical][compartments][hardblocker]
So this bug as filed has been fixed, there's remaining bugs on making them default safe (bug 628410).

Marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Is the fact that bug 572129 is not fixed have any bearing on whether we want to consider this fixed? See https://bugzilla.mozilla.org/show_bug.cgi?id=572129#c23.
If we're going to fix this on 1.9.2 it needs to be now. If we're not ever going to do it we should just say that.
blocking1.9.1: needed → -
blocking1.9.2: needed → .20+
I don't think we can really fix this on 1.9.2. COWs (as we were hoping to backport them when this bug was filed) have been replaced by new code that is heavily dependent on compartments.
blocking1.9.2: .20+ → needed
blocking1.9.2: needed → -
status1.9.1: wanted → wontfix
status1.9.2: wanted → wontfix

Updated

3 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.