bug Sylius#13765 [Security] Fixes for SVG XSS, wrong cache for logged…

… in users and clickjacking (ernestWarwas, lchrusciel, GSadee, Zales0123, Rafikooo)

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 | 
| 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)

<!--
 - 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
67de9e8 bug #14 [Security] Clickjacking vulnerability fixed (ernestWarwas)
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
253f66b bug #11 [Security] Set cache control directives to fix security leak after logging out and using back button (lchrusciel, GSadee)
46ed54b [Security] XSS - SVG file upload vulnerability fixed
6ccc2d6 bug #12 [Security] XSS - SVG file upload vulnerability fixed (Rafikooo)

composer.json

@@ -35,6 +35,7 @@
         "doctrine/migrations": "^3.0",
         "doctrine/orm": "^2.7",
         "egulias/email-validator": "^2.1",
+        "enshrined/svg-sanitize": "^0.15.4",
         "fakerphp/faker": "^1.9",
         "friendsofphp/proxy-manager-lts": "^1.0",
         "friendsofsymfony/oauth-server-bundle": ">2.0.0-alpha.0 ^2.0@dev",

features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature

@@ -0,0 +1,17 @@
+@customer_account
+Feature: Securing access to the account after using the back button after logging out
+    In order to have my personal information secured
+    As a Customer
+    I want to be unable to access to the account by using the back button after logging out
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And I am a logged in customer
+        And I am browsing my orders
+
+    @ui @javascript @no-api
+    Scenario: Securing access to the account after using the back button after logging out
+        When I log out
+        And I go back one page in the browser
+        Then I should not see my orders
+        And I should be on the login page

features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature

@@ -0,0 +1,17 @@
+@admin_dashboard
+Feature: Securing access to the administration panel after using the back button after logging out
+    In order to have administration panel secured
+    As an Administrator
+    I want to be unable to access to the administration panel by using the back button after logging out
+
+    Background:
+        Given the store operates on a single channel in "United States"
+        And I am logged in as an administrator
+        And I am on the administration dashboard
+
+    @ui @javascript @no-api
+    Scenario: Securing access to administration dashboard after using the back button after logging out
+        When I log out
+        And I go back one page in the browser
+        Then I should not see the administration dashboard
+        And I should be on the login page

src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php

@@ -30,9 +30,10 @@ public function __construct(DashboardPageInterface $dashboardPage)
     }
 
     /**
+     * @Given I am on the administration dashboard
      * @When I (try to )open administration dashboard
      */
-    public function iOpenAdministrationDashboard()
+    public function iOpenAdministrationDashboard(): void
     {
         try {
             $this->dashboardPage->open();
@@ -56,6 +57,14 @@ public function iChooseChannel($channelName)
         $this->dashboardPage->chooseChannel($channelName);
     }
 
+    /**
+     * @When I log out
+     */
+    public function iLogOut(): void
+    {
+        $this->dashboardPage->logOut();
+    }
+
     /**
      * @Then I should see :number new orders
      */
@@ -103,4 +112,12 @@ public function iShouldSeeNewOrdersInTheList($number)
     {
         Assert::same($this->dashboardPage->getNumberOfNewOrdersInTheList(), (int) $number);
     }
+
+    /**
+     * @Then I should not see the administration dashboard
+     */
+    public function iShouldNotSeeTheAdministrationDashboard(): void
+    {
+        Assert::false($this->dashboardPage->isOpen());
+    }
 }

src/Sylius/Behat/Context/Ui/Admin/LoginContext.php

@@ -141,4 +141,12 @@ private function logInAgain($username, $password)
         $this->loginPage->specifyPassword($password);
         $this->loginPage->logIn();
     }
+
+    /**
+     * @Then I should be on the login page
+     */
+    public function iShouldBeOnTheLoginPage(): void
+    {
+        Assert::true($this->loginPage->isOpen());
+    }
 }

src/Sylius/Behat/Context/Ui/BrowserContext.php

@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Behat\Context\Ui;
+
+use Behat\Behat\Context\Context;
+use Sylius\Behat\Element\BrowserElementInterface;
+
+final class BrowserContext implements Context
+{
+    /** @var BrowserElementInterface */
+    private $browserElement;
+
+    public function __construct(BrowserElementInterface $browserElement)
+    {
+        $this->browserElement = $browserElement;
+    }
+
+    /**
+     * @When I go back one page in the browser
+     */
+    public function iGoBackOnePageInTheBrowser(): void
+    {
+        $this->browserElement->goBack();
+    }
+}

src/Sylius/Behat/Context/Ui/Shop/AccountContext.php

@@ -263,9 +263,10 @@ public function iShouldBeNotifiedThatThePasswordShouldBeAtLeastCharactersLong()
     }
 
     /**
+     * @Given I am browsing my orders
      * @When I browse my orders
      */
-    public function iBrowseMyOrders()
+    public function iBrowseMyOrders(): void
     {
         $this->orderIndexPage->open();
     }
@@ -531,4 +532,20 @@ public function iShouldNotBeLoggedIn(): void
 
         throw new \InvalidArgumentException('Dashboard has been openned, but it shouldn\'t as customer should not be logged in');
     }
+
+    /**
+     * @Then I should not see my orders
+     */
+    public function iShouldNotSeeMyOrders(): void
+    {
+        Assert::false($this->orderIndexPage->isOpen());
+    }
+
+    /**
+     * @Then I should be on the login page
+     */
+    public function iShouldBeOnTheLoginPage(): void
+    {
+        Assert::true($this->loginPage->isOpen());
+    }
 }

src/Sylius/Behat/Element/BrowserElement.php

@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Behat\Element;
+
+use FriendsOfBehat\PageObjectExtension\Element\Element;
+
+final class BrowserElement extends Element implements BrowserElementInterface
+{
+    public function goBack(): void
+    {
+        $this->getDriver()->back();
+    }
+}

src/Sylius/Behat/Element/BrowserElementInterface.php

@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Behat\Element;
+
+interface BrowserElementInterface
+{
+    public function goBack(): void;
+}

src/Sylius/Behat/Resources/config/services/contexts/ui.xml

@@ -299,6 +299,10 @@
             <tag name="fob.context_service" />
         </service>
 
+        <service id="sylius.behat.context.ui.browser" class="Sylius\Behat\Context\Ui\BrowserContext">
+            <argument type="service" id="sylius.behat.element.browser" />
+        </service>
+
         <service id="sylius.behat.context.ui.channel" class="Sylius\Behat\Context\Ui\ChannelContext">
             <argument type="service" id="sylius.behat.shared_storage" />
             <argument type="service" id="sylius.behat.channel_context_setter" />

src/Sylius/Behat/Resources/config/services/elements.xml

@@ -20,10 +20,13 @@
         <import resource="elements/shop.xml" />
         <import resource="elements/product.xml" />
     </imports>
+
     <services>
         <service id="sylius.behat.element" class="FriendsOfBehat\PageObjectExtension\Element\Element" abstract="true" public="false">
             <argument type="service" id="behat.mink.default_session" />
             <argument type="service" id="behat.mink.parameters" />
         </service>
+
+        <service id="sylius.behat.element.browser" class="Sylius\Behat\Element\BrowserElement" parent="sylius.behat.element" public="false" />
     </services>
 </container>

src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml

@@ -33,6 +33,7 @@ default:
                 - sylius.behat.context.setup.user
                 - sylius.behat.context.setup.zone
 
+                - sylius.behat.context.ui.browser
                 - sylius.behat.context.ui.channel
                 - sylius.behat.context.ui.email
                 - sylius.behat.context.ui.shop.account
@@ -44,6 +45,7 @@ default:
                 - sylius.behat.context.ui.shop.checkout.shipping
                 - sylius.behat.context.ui.shop.currency
                 - sylius.behat.context.ui.shop.homepage
+                - sylius.behat.context.ui.user
 
             filters:
                 tags: "@customer_account&&@ui"

src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml

@@ -26,7 +26,9 @@ default:
                 - sylius.behat.context.transform.shared_storage
 
                 - sylius.behat.context.ui.admin.dashboard
+                - sylius.behat.context.ui.admin.login
                 - sylius.behat.context.ui.admin.notification
+                - sylius.behat.context.ui.browser
 
             filters:
                 tags: "@admin_dashboard&&@ui"

src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php

@@ -0,0 +1,52 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace Sylius\Bundle\AdminBundle\EventListener;
+
+use Sylius\Bundle\AdminBundle\SectionResolver\AdminSection;
+use Sylius\Bundle\CoreBundle\SectionResolver\SectionProviderInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpKernel\Event\ResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+final class AdminSectionCacheControlSubscriber implements EventSubscriberInterface
+{
+    /** @var SectionProviderInterface */
+    private $sectionProvider;
+
+    public function __construct(SectionProviderInterface $sectionProvider)
+    {
+        $this->sectionProvider = $sectionProvider;
+    }
+
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            KernelEvents::RESPONSE => 'setCacheControlDirectives',
+        ];
+    }
+
+    public function setCacheControlDirectives(ResponseEvent $event): void
+    {
+        if (!$this->sectionProvider->getSection() instanceof AdminSection) {
+            return;
+        }
+
+        $response = $event->getResponse();
+
+        $response->headers->addCacheControlDirective('no-cache', true);
+        $response->headers->addCacheControlDirective('max-age', '0');
+        $response->headers->addCacheControlDirective('must-revalidate', true);
+        $response->headers->addCacheControlDirective('no-store', true);
+    }
+}

src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml

@@ -25,5 +25,10 @@
             <argument type="service" id="session" />
             <tag name="kernel.event_subscriber" event="kernel.exception" />
         </service>
+
+        <service id="sylius.event_subscriber.admin_cache_control_subscriber" class="Sylius\Bundle\AdminBundle\EventListener\AdminSectionCacheControlSubscriber">
+            <argument type="service" id="sylius.section_resolver.uri_based_section_resolver" />
+            <tag name="kernel.event_subscriber" event="kernel.response" />
+        </service>
     </services>
 </container>