Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security] Fixes for SVG XSS, wrong cache for logged in users and clickjacking #13765

Merged
merged 14 commits into from
Mar 14, 2022

Conversation

lchrusciel
Copy link
Member

Q A
Branch? 1.9
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets
License MIT

This PR aims to solve 3 issues:

  1. Possibility to inject SVGs with scripts (https://rietta.com/blog/svg-xss-injection-attacks/)
  2. Possibility to check admin pages with back button after logging out due to wrong cache'ing configuration
  3. Clickjacking while rendering Sylius in iframe (https://portswigger.net/web-security/clickjacking)

ernestWarwas and others added 14 commits March 7, 2022 11:09
This PR was merged into the 1.9 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.9
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| License         | MIT

There was a possibility to load a page within an iframe which is enabling to the possibility to perform a clickjacking attack.

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

0886078 listener added to finish response with X-Frame-Options sameorigin header
c236431 suggested review changes
…after logging out and using back button (lchrusciel, GSadee)

This PR was merged into the 1.9 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.9
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | GHSA-7563-75j9-6h5p
| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

4b6a77a [UI] Force no-store cache directives for admin and customer account section
691b700 [Maintenace] Test existence of new cache headers
08d0f5a Remove type declarations for properties due to supporting PHP 7.3
94366fd Minor fixes for specs and unit tests of cache control subscribers
5dee3dc [Behat] Add scenarios for securing access to account and dashboard after logging out
d4bf36c [Behat] Extract browser element and context
afa04e3 Replace str_contains with strpos method to support PHP 7
b00eb51 [PHPUnit] Move subscribers tests to main directory
This PR was merged into the 1.9 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.9 <!-- see the comment below -->
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no <!-- don't forget to update the UPGRADE-*.md file -->
| Related tickets | GHSA-4qrp-27r3-66fj
| License         | MIT

There is a possibility to upload an SVG file containing XSS code in the admin panel. In order to perform an XSS attack, the file itself has to be open in a new card (or loaded outside of the IMG tag). The problem applies both to the files opened on the admin panel and shop pages.

<!--
 - Bug fixes must be submitted against the 1.10 or 1.11 branch(the lowest possible)
 - Features and deprecations must be submitted against the master branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

46ed54b [Security] XSS - SVG file upload vulnerability fixed
@lchrusciel lchrusciel requested a review from a team as a code owner March 14, 2022 13:41
@probot-autolabeler probot-autolabeler bot added Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Shop ShopBundle related issues and PRs. labels Mar 14, 2022
@lchrusciel lchrusciel added Critical Issues and PRs, which are critical and should be fixed ASAP. Bug Confirmed bugs or bugfixes. labels Mar 14, 2022
@Zales0123 Zales0123 merged commit 3da169e into Sylius:1.9 Mar 14, 2022
@Zales0123
Copy link
Member

Thanks, Łukasz! 🎉

@lchrusciel lchrusciel deleted the security-fix branch March 14, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Bug Confirmed bugs or bugfixes. Critical Issues and PRs, which are critical and should be fixed ASAP. Shop ShopBundle related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants