Meta refresh via XSLT stylesheet bypasses <iframe sandbox> attribute restrictions
Categories
(Core :: XSLT, defect)
Tracking
()
People
(Reporter: peterv, Assigned: peterv)
References
Details
(Keywords: sec-high, Whiteboard: [sec-survey][adv-main96+][adv-ESR91.5+][post-critsmash-triage])
Attachments
(5 files, 1 obsolete file)
48 bytes,
text/x-phabricator-request
|
tjr
:
sec-approval+
|
Details | Review |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
4.62 KB,
patch
|
RyanVM
:
approval-mozilla-esr91+
|
Details | Diff | Splinter Review |
4.52 KB,
patch
|
diannaS
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
150 bytes,
text/plain
|
Details |
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
Depends on D134222
Comment 3•3 years ago
|
||
Calling this sec-high, because we called bug 1729517 a sec-high.
Comment 4•3 years ago
|
||
Btw, good catch peter. Thanks!
Assignee | ||
Comment 5•3 years ago
|
||
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.
Assignee | ||
Updated•3 years ago
|
Comment 6•3 years ago
|
||
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•3 years ago
|
||
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•3 years ago
|
||
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•3 years ago
|
||
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•3 years ago
|
||
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.
Assignee | ||
Comment 11•3 years ago
|
||
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
Assignee | ||
Comment 12•3 years ago
|
||
[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.
Assignee | ||
Comment 13•3 years ago
|
||
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•3 years ago
•
|
||
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•3 years ago
|
||
uplift |
Comment 16•3 years ago
|
||
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•3 years ago
|
||
uplift |
Updated•3 years ago
|
Comment 18•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 19•3 years ago
|
||
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
Updated•2 years ago
|
Description
•