Closed Bug 1481640 Opened 2 years ago Closed 1 month ago

Get rid of forcePermissiveCOWs()

Categories

(Core :: XPConnect, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox78 --- fixed
firefox79 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

Details

Attachments

(1 file)

Keeping this special case around in our wrapper management code is problematic enough on its own, but I also think it's a footgun, at this point.

In bug 1481021, I stopped SpecialPowers from setting the flag on frame script globals for unrelated reasons, and found lots of code improperly relying on the fact that it allowed them to pass objects from frame script globals to unprivileged content. Most of that code seemed to be test-only, and therefore not a huge concern, but it's only a matter of time before some production code starts relying on it somewhere as well, and therefore working fine when run under a test harness, but failing in the real world.

And, in fact, that very thing happened a few days ago:

https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/browser/components/payments/content/paymentDialogFrameScript.js#92-104

If we want to expose any privileged objects to unprivileged scopes, at this point, we should explicitly clone or wrap all of them. SpecialPowers.wrap should work well enough for the existing cases I know of in tests, as long as we create the Proxy object in the content compartment rather than the JSM.
Priority: -- → P3
Depends on: 1482279
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/490b19dd5762
Remove forcePermissiveCOWs(). r=kmag
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

Comment on attachment 9153020 [details]
Bug 1481640 - Remove forcePermissiveCOWs(). r?kmag

Beta/Release Uplift Approval Request

  • User impact if declined: More potential attack vector
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is test-only dead code removal.
  • String changes made/needed: none
Attachment #9153020 - Flags: approval-mozilla-beta?

Comment on attachment 9153020 [details]
Bug 1481640 - Remove forcePermissiveCOWs(). r?kmag

ok, I guess. approved for 78.0b2

Attachment #9153020 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.