1746720 - (CVE-2021-4140) Meta refresh via XSLT stylesheet bypasses <iframe sandbox> attribute restrictions

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: Bug 1746720 - Don't special-case <meta> refresh for XSLT. r?ckerschb!

Comment 2

[Peter Van der Beken [:peterv]]

Attachment: Bug 1746720 - Don't special-case <meta> refresh for XSLT - test. r?ckerschb!

Depends on D134222

Comment 3

[Frederik Braun [:freddy]]

Calling this sec-high, because we called bug 1729517 a sec-high.

Comment 4

[Frederik Braun [:freddy]]

Btw, good catch peter. Thanks!

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 9256030 [details]
Bug 1746720 - Don't special-case <meta> refresh for XSLT. r?ckerschb!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I don't think it's very obvious, since I'm just removing some code. It is obvious that it has to do with XSLT and meta refresh, but nothing points to a problem with sandboxing.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which older supported branches are affected by this flaw?: All
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Patch should just apply.
• How likely is this patch to cause regressions; how much testing does it need?: Pretty sure it won't cause regressions, we just make meta refresh with XSLT have the same behaviour as for non-XSLT loads.

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9256030 [details]
Bug 1746720 - Don't special-case <meta> refresh for XSLT. r?ckerschb!

approved to land and request uplift

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9256031 [details]
Bug 1746720 - Don't special-case <meta> refresh for XSLT - test. r?ckerschb!

We can land this after Jan 24

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb
https://hg.mozilla.org/integration/autoland/rev/06f49bcfb639620f443deca490c2158ce1f23a2c
https://hg.mozilla.org/mozilla-central/rev/06f49bcfb639

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This needs rebased patches for both Beta and ESR91. Please attach those and request approval ASAP as we're down to our final of the beta of the cycle.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 11

[Peter Van der Beken [:peterv]]

Attachment: beta - Bug 1746720 - Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb

Approval Request Comment
[Feature/Bug causing the regression]: XSLT
[User impact if declined]: security bug, allows to circumvent a iframe sandbox restriction
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: just removes some code for XSLT, falling back to more secure code that was already used for the non-XSLT case.
[String changes made/needed]: none

Comment 12

[Peter Van der Beken [:peterv]]

Attachment: esr91 - - Bug 1746720 - Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: allows to circumvent a iframe sandbox restriction
Fix Landed on Version: 97
Risk to taking this patch (and alternatives if risky): low risk, just removes some code for XSLT, falling back to more secure code that was already used for the non-XSLT case.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 13

[Peter Van der Beken [:peterv]]

Attachment: beta - Bug 1746720 - Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb

Approval Request Comment
[Feature/Bug causing the regression]: XSLT
[User impact if declined]: security bug, allows to circumvent a iframe sandbox restriction
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: just removes some code for XSLT, falling back to more secure code that was already used for the non-XSLT case.
[String changes made/needed]: none

Comment 14

[Dianna Smith [:diannaS]]

Comment on attachment 9257170 [details] [diff] [review]
beta - Bug 1746720 - Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb

Approved for 96.0rc1

Comment 15

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c410b5ad433c

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9256696 [details] [diff] [review]
esr91 - - Bug 1746720 - Don't special-case <meta> refresh for XSLT. r=ckerschb,freddyb

Approved for 91.5esr.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/5a03174e6432

Comment 18

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Don't special-case <meta> refresh for XSLT - test. r=ckerschb,freddyb
https://hg.mozilla.org/integration/autoland/rev/a833b05ff6afade765ba23bda0eec6dd74513bae
https://hg.mozilla.org/mozilla-central/rev/a833b05ff6af