Closed Bug 1292369 Opened 3 years ago Closed 3 years ago

It's too easy to access content windows that point to a different inner window than we expect

Categories

(WebExtensions :: Untriaged, defect, P1)

defect

Tracking

(firefox49+ wontfix, firefox50 fixed, firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox49 + wontfix
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

Details

(Whiteboard: triaged)

Attachments

(1 file)

No description provided.
Priority: -- → P1
Whiteboard: triaged
Comment on attachment 8777998 [details]
Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to.

https://reviewboard.mozilla.org/r/69382/#review68514

::: toolkit/components/extensions/ExtensionUtils.jsm:166
(Diff revision 1)
>      this.jsonSandbox = null;
>      this.active = true;
> +
> +    this.docShell = null;
> +    this.contentWindow = null;
> +    this.innerWindowID = null;

Initializing to 0 is better.
Attachment #8777998 - Flags: review?(wmccloskey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c4fcb84ee3930af35eb6cb83c044d6ab8a08abb
Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to. r=billm
https://hg.mozilla.org/mozilla-central/rev/4c4fcb84ee39
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1292332
Duplicate of this bug: 1292344
Blocks: 1288901
Duplicate of this bug: 1302406
Will this bug also be fixed in Firefox 50? 
We need a fix for 1302406. Otherwise we have to build a workaround in our WebExtension only for FF50.
Comment on attachment 8777998 [details]
Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: The previous code has potential severe security implications, and also leads inconsistent user-facing behavior when pages containing extension content scripts or CSS navigate backwards or forwards.
[Describe test coverage new/current, TreeHerder]: The behaviors addressed by this patch are covered by both functional and low-level unit tests.
[Risks and why]: Low. These changes are fairly minimal, and are aimed entirely at addressing existing inconsistent and insecure behavior. Any potential new inconsistencies are likely to be less severe than the ones that are being fixed.
[String/UUID change made/needed]: None.
Attachment #8777998 - Flags: approval-mozilla-beta?
Attachment #8777998 - Flags: approval-mozilla-aurora?
(Note: Flagging for beta approval for 50, in case this isn't handled before the next merge. I don't think it should land in 49 this late in the RC cycle.)
Comment on attachment 8777998 [details]
Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to.

webextensions related, Aurora50+
Attachment #8777998 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8777998 - Flags: approval-mozilla-beta?
See Also: → 1462121
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.