diff --git a/composer.json b/composer.json index e750eb0d0aa..9e3de1f2245 100644 --- a/composer.json +++ b/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", diff --git a/features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature b/features/account/customer_account/securing_access_to_account_after_using_back_button_after_logging_out.feature new file mode 100644 index 00000000000..2861b65eff4 --- /dev/null +++ b/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 diff --git a/features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature b/features/admin/securing_access_to_account_after_using_back_button_after_logging_out.feature new file mode 100644 index 00000000000..8954ec9a5ff --- /dev/null +++ b/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 diff --git a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php index 0a76bdda944..52b281e79d1 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php +++ b/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()); + } } diff --git a/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php b/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php index a8434c56c22..3064c2e141e 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php +++ b/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()); + } } diff --git a/src/Sylius/Behat/Context/Ui/BrowserContext.php b/src/Sylius/Behat/Context/Ui/BrowserContext.php new file mode 100644 index 00000000000..544704d13d5 --- /dev/null +++ b/src/Sylius/Behat/Context/Ui/BrowserContext.php @@ -0,0 +1,36 @@ +browserElement = $browserElement; + } + + /** + * @When I go back one page in the browser + */ + public function iGoBackOnePageInTheBrowser(): void + { + $this->browserElement->goBack(); + } +} diff --git a/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php b/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php index ecb99ee819f..79e4e515a51 100644 --- a/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php +++ b/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()); + } } diff --git a/src/Sylius/Behat/Element/BrowserElement.php b/src/Sylius/Behat/Element/BrowserElement.php new file mode 100644 index 00000000000..3743fa94fc7 --- /dev/null +++ b/src/Sylius/Behat/Element/BrowserElement.php @@ -0,0 +1,24 @@ +getDriver()->back(); + } +} diff --git a/src/Sylius/Behat/Element/BrowserElementInterface.php b/src/Sylius/Behat/Element/BrowserElementInterface.php new file mode 100644 index 00000000000..4f533b13d57 --- /dev/null +++ b/src/Sylius/Behat/Element/BrowserElementInterface.php @@ -0,0 +1,19 @@ + + + + + diff --git a/src/Sylius/Behat/Resources/config/services/elements.xml b/src/Sylius/Behat/Resources/config/services/elements.xml index 659d57b131f..36d55417c4e 100644 --- a/src/Sylius/Behat/Resources/config/services/elements.xml +++ b/src/Sylius/Behat/Resources/config/services/elements.xml @@ -20,10 +20,13 @@ + + + diff --git a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml b/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml index 34f08b62f9a..12b5861363c 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/account/customer.yml +++ b/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" diff --git a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml b/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml index 2b2f32caa1f..0a3844c97fc 100644 --- a/src/Sylius/Behat/Resources/config/suites/ui/admin/dashboard.yml +++ b/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" diff --git a/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php new file mode 100644 index 00000000000..c85a01fbff1 --- /dev/null +++ b/src/Sylius/Bundle/AdminBundle/EventListener/AdminSectionCacheControlSubscriber.php @@ -0,0 +1,52 @@ +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); + } +} diff --git a/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml b/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml index 8c074404ec3..3ae6fdbe3c4 100644 --- a/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml +++ b/src/Sylius/Bundle/AdminBundle/Resources/config/services/listener.xml @@ -25,5 +25,10 @@ + + + + + diff --git a/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php new file mode 100644 index 00000000000..f67decf3f5d --- /dev/null +++ b/src/Sylius/Bundle/AdminBundle/spec/EventListener/AdminSectionCacheControlSubscriberSpec.php @@ -0,0 +1,93 @@ +beConstructedWith($sectionProvider); + } + + function it_subscribes_to_kernel_response_event(): void + { + $this::getSubscribedEvents()->shouldReturn([KernelEvents::RESPONSE => 'setCacheControlDirectives']); + } + + function it_adds_cache_control_directives_to_admin_requests( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + AdminSection $adminSection + ): void { + $sectionProvider->getSection()->willReturn($adminSection); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldBeCalled(); + + $this->setCacheControlDirectives($event); + } + + function it_does_nothing_if_section_is_different_than_admin( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + SectionInterface $section + ): void { + $sectionProvider->getSection()->willReturn($section); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldNotBeCalled(); + + $this->setCacheControlDirectives($event); + } +} diff --git a/src/Sylius/Bundle/ApiBundle/composer.json b/src/Sylius/Bundle/ApiBundle/composer.json index 6a4c4fb5013..fa47c64d68a 100644 --- a/src/Sylius/Bundle/ApiBundle/composer.json +++ b/src/Sylius/Bundle/ApiBundle/composer.json @@ -26,6 +26,7 @@ "php": "^7.3", "api-platform/core": "^2.5", "doctrine/dbal": "^2.7", + "enshrined/svg-sanitize": "^0.15.4", "lexik/jwt-authentication-bundle": "^2.6", "sylius/core-bundle": "^1.7", "symfony/messenger": "^4.4 || ^5.2" diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php b/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php new file mode 100644 index 00000000000..8e4727682ea --- /dev/null +++ b/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php @@ -0,0 +1,48 @@ + 'onKernelResponse', + ]; + } + + public function onKernelResponse(ResponseEvent $event): void + { + if (!$this->isMainRequest($event)) { + return; + } + + $response = $event->getResponse(); + + $response->headers->set('X-Frame-Options', 'sameorigin'); + } + + private function isMainRequest(ResponseEvent $event): bool + { + if (\method_exists($event, 'isMainRequest')) { + return $event->isMainRequest(); + } + + return $event->isMasterRequest(); + } +} diff --git a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml index ef39f9b6ba5..9f02b50e081 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml @@ -95,6 +95,10 @@ + + + + diff --git a/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php new file mode 100644 index 00000000000..803d481c9fa --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php @@ -0,0 +1,52 @@ +sectionProvider = $sectionProvider; + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::RESPONSE => 'setCacheControlDirectives', + ]; + } + + public function setCacheControlDirectives(ResponseEvent $event): void + { + if (!$this->sectionProvider->getSection() instanceof ShopCustomerAccountSubSection) { + 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); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml b/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml index 5b5e5d86234..476ea0be3ea 100644 --- a/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml +++ b/src/Sylius/Bundle/ShopBundle/Resources/config/services.xml @@ -30,6 +30,7 @@ + account diff --git a/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml b/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml index 0fb75b33793..f4467d7c00b 100644 --- a/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml @@ -25,6 +25,11 @@ + + + + + diff --git a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php new file mode 100644 index 00000000000..c99afa65ec5 --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php @@ -0,0 +1,18 @@ +shopCustomerAccountUri = $shopCustomerAccountUri; + } + public function getSection(string $uri): SectionInterface { + if (strpos($uri, $this->shopCustomerAccountUri) !== false) { + return new ShopCustomerAccountSubSection(); + } + return new ShopSection(); } } diff --git a/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php new file mode 100644 index 00000000000..61fc6e72b81 --- /dev/null +++ b/src/Sylius/Bundle/ShopBundle/spec/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberSpec.php @@ -0,0 +1,93 @@ +beConstructedWith($sectionProvider); + } + + function it_subscribes_to_kernel_response_event() + { + $this::getSubscribedEvents()->shouldReturn([KernelEvents::RESPONSE => 'setCacheControlDirectives']); + } + + function it_adds_cache_control_directives_to_customer_account_requests( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + ShopCustomerAccountSubSection $customerAccountSubSection + ): void { + $sectionProvider->getSection()->willReturn($customerAccountSubSection); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldBeCalled(); + + $this->setCacheControlDirectives($event); + } + + function it_does_nothing_if_section_is_different_than_customer_account( + SectionProviderInterface $sectionProvider, + HttpKernelInterface $kernel, + Request $request, + Response $response, + ResponseHeaderBag $responseHeaderBag, + SectionInterface $section + ): void { + $sectionProvider->getSection()->willReturn($section); + + $response->headers = $responseHeaderBag->getWrappedObject(); + + $event = new ResponseEvent( + $kernel->getWrappedObject(), + $request->getWrappedObject(), + KernelInterface::MASTER_REQUEST, + $response->getWrappedObject() + ); + + $responseHeaderBag->addCacheControlDirective('no-cache', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('max-age', '0')->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('must-revalidate', true)->shouldNotBeCalled(); + $responseHeaderBag->addCacheControlDirective('no-store', true)->shouldNotBeCalled(); + + $this->setCacheControlDirectives($event); + } +} diff --git a/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php b/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php index ff710236909..1d687b94d71 100644 --- a/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php +++ b/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php @@ -15,6 +15,7 @@ use PhpSpec\ObjectBehavior; use Sylius\Bundle\CoreBundle\SectionResolver\UriBasedSectionResolverInterface; +use Sylius\Bundle\ShopBundle\SectionResolver\ShopCustomerAccountSubSection; use Sylius\Bundle\ShopBundle\SectionResolver\ShopSection; final class ShopUriBasedSectionResolverSpec extends ObjectBehavior @@ -24,7 +25,7 @@ function it_it_uri_based_section_resolver(): void $this->shouldImplement(UriBasedSectionResolverInterface::class); } - function it_always_returns_shop(): void + function it_returns_shop_by_default(): void { $this->getSection('/api/something')->shouldBeLike(new ShopSection()); $this->getSection('/api')->shouldBeLike(new ShopSection()); @@ -33,4 +34,29 @@ function it_always_returns_shop(): void $this->getSection('/admin/asd')->shouldBeLike(new ShopSection()); $this->getSection('/en_US/api')->shouldBeLike(new ShopSection()); } + + function it_uses_account_prefix_for_customer_account_subsection_by_default(): void + { + $this->getSection('/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/api/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/en_US/account')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/account/random')->shouldBeLike(new ShopCustomerAccountSubSection()); + } + + function it_may_have_account_prefix_customized(): void + { + $this->beConstructedWith('konto'); + + $this->getSection('/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/api/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/en_US/konto')->shouldBeLike(new ShopCustomerAccountSubSection()); + $this->getSection('/konto/random')->shouldBeLike(new ShopCustomerAccountSubSection()); + + $this->getSection('/account')->shouldBeLike(new ShopSection()); + $this->getSection('/account')->shouldBeLike(new ShopSection()); + $this->getSection('/api/account')->shouldBeLike(new ShopSection()); + $this->getSection('/en_US/account')->shouldBeLike(new ShopSection()); + $this->getSection('/account/random')->shouldBeLike(new ShopSection()); + } } diff --git a/src/Sylius/Component/Core/Uploader/ImageUploader.php b/src/Sylius/Component/Core/Uploader/ImageUploader.php index 495caf06916..21d5302ce5d 100644 --- a/src/Sylius/Component/Core/Uploader/ImageUploader.php +++ b/src/Sylius/Component/Core/Uploader/ImageUploader.php @@ -13,6 +13,7 @@ namespace Sylius\Component\Core\Uploader; +use enshrined\svgSanitize\Sanitizer; use Gaufrette\Filesystem; use Sylius\Component\Core\Generator\ImagePathGeneratorInterface; use Sylius\Component\Core\Generator\UploadedImagePathGenerator; @@ -22,12 +23,18 @@ class ImageUploader implements ImageUploaderInterface { + private const MIME_SVG_XML = 'image/svg+xml'; + private const MIME_SVG = 'image/svg'; + /** @var Filesystem */ protected $filesystem; /** @var ImagePathGeneratorInterface */ protected $imagePathGenerator; + /** @var Sanitizer */ + protected $sanitizer; + public function __construct( Filesystem $filesystem, ?ImagePathGeneratorInterface $imagePathGenerator = null @@ -41,6 +48,7 @@ public function __construct( } $this->imagePathGenerator = $imagePathGenerator ?? new UploadedImagePathGenerator(); + $this->sanitizer = new Sanitizer(); } public function upload(ImageInterface $image): void @@ -49,11 +57,13 @@ public function upload(ImageInterface $image): void return; } + /** @var File $file */ $file = $image->getFile(); - /** @var File $file */ Assert::isInstanceOf($file, File::class); + $fileContent = $this->sanitizeContent(file_get_contents($file->getPathname()), $file->getMimeType()); + if (null !== $image->getPath() && $this->has($image->getPath())) { $this->remove($image->getPath()); } @@ -64,10 +74,7 @@ public function upload(ImageInterface $image): void $image->setPath($path); - $this->filesystem->write( - $image->getPath(), - file_get_contents($image->getFile()->getPathname()) - ); + $this->filesystem->write($image->getPath(), $fileContent); } public function remove(string $path): bool @@ -79,6 +86,15 @@ public function remove(string $path): bool return false; } + protected function sanitizeContent(string $fileContent, string $mimeType): string + { + if (self::MIME_SVG_XML === $mimeType || self::MIME_SVG === $mimeType) { + $fileContent = $this->sanitizer->sanitize($fileContent); + } + + return $fileContent; + } + private function has(string $path): bool { return $this->filesystem->has($path); diff --git a/src/Sylius/Component/Core/composer.json b/src/Sylius/Component/Core/composer.json index d656d92d3eb..ac5a6bab7e3 100644 --- a/src/Sylius/Component/Core/composer.json +++ b/src/Sylius/Component/Core/composer.json @@ -27,6 +27,7 @@ ], "require": { "php": "^7.3", + "enshrined/svg-sanitize": "^0.15.4", "knplabs/gaufrette": "^0.8", "payum/payum": "^1.6", "php-http/guzzle6-adapter": "^2.0", diff --git a/symfony.lock b/symfony.lock index cf7e64da40d..9c829d7ae67 100644 --- a/symfony.lock +++ b/symfony.lock @@ -155,6 +155,9 @@ "egulias/email-validator": { "version": "2.1.19" }, + "enshrined/svg-sanitize": { + "version": "0.15.4" + }, "fakerphp/faker": { "version": "v1.12.0" }, diff --git a/tests/Controller/XFrameOptionsTest.php b/tests/Controller/XFrameOptionsTest.php new file mode 100644 index 00000000000..1011230418b --- /dev/null +++ b/tests/Controller/XFrameOptionsTest.php @@ -0,0 +1,29 @@ +client->request('GET', '/'); + + $response = $this->client->getResponse(); + + $this->assertSame('sameorigin', $response->headers->get('X-Frame-Options')); + } +} diff --git a/tests/EventListener/AdminSectionCacheControlSubscriberTest.php b/tests/EventListener/AdminSectionCacheControlSubscriberTest.php new file mode 100644 index 00000000000..ae9bd10c944 --- /dev/null +++ b/tests/EventListener/AdminSectionCacheControlSubscriberTest.php @@ -0,0 +1,31 @@ +request('GET', '/admin/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); + } +} diff --git a/tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php b/tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php new file mode 100644 index 00000000000..c3ad2f1da82 --- /dev/null +++ b/tests/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriberTest.php @@ -0,0 +1,31 @@ +request('GET', '/en_US/account/'); + + $this->assertResponseHeaderSame('Cache-Control', 'max-age=0, must-revalidate, no-cache, no-store, private'); + } +} diff --git a/tests/Functional/ImageUploaderTest.php b/tests/Functional/ImageUploaderTest.php new file mode 100644 index 00000000000..3bc4cecef14 --- /dev/null +++ b/tests/Functional/ImageUploaderTest.php @@ -0,0 +1,59 @@ +getContainer(); + + $imageUploader = self::$container->get('sylius.image_uploader'); + $fileSystem = self::$container->get('gaufrette.sylius_image_filesystem'); + + $file = new UploadedFile(__DIR__ . '/../Resources/xss.svg', 'xss.svg'); + Assert::assertStringContainsString('getContent($file)); + + $image = new ProductImage(); + $image->setFile($file); + + $imageUploader->upload($image); + + $sanitizedFile = $fileSystem->get($image->getPath()); + Assert::assertStringNotContainsString('getContent()); + } + + private function getContent(UploadedFile $file): string + { + $content = file_get_contents($file->getPathname()); + + if (false === $content) { + throw new FileException(sprintf('Could not get the content of the file "%s".', $file->getPathname())); + } + + return $content; + } +} diff --git a/tests/Resources/xss.svg b/tests/Resources/xss.svg new file mode 100644 index 00000000000..08ecd246d46 --- /dev/null +++ b/tests/Resources/xss.svg @@ -0,0 +1,9 @@ + + + + + + +