diff --git a/composer.json b/composer.json index 199ab1f996..e2a383932c 100644 --- a/composer.json +++ b/composer.json @@ -43,6 +43,7 @@ "doctrine/orm": "^2.7", "doctrine/persistence": "^2.3", "egulias/email-validator": "^3.1", + "enshrined/svg-sanitize": "^0.15.4", "fakerphp/faker": "^1.9", "friendsofphp/proxy-manager-lts": "^1.0", "friendsofsymfony/rest-bundle": "^3.0", 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 0000000000..2861b65eff --- /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 0000000000..8954ec9a5f --- /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 319a3cf401..853c112e32 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php +++ b/src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php @@ -29,9 +29,10 @@ final class DashboardContext implements Context } /** + * @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(); @@ -55,6 +56,14 @@ final class DashboardContext implements Context $this->dashboardPage->chooseChannel($channelName); } + /** + * @When I log out + */ + public function iLogOut(): void + { + $this->dashboardPage->logOut(); + } + /** * @Then I should see :number new orders */ @@ -102,4 +111,12 @@ final class DashboardContext implements Context { 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 337a4c801f..ba3e1d2a21 100644 --- a/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php +++ b/src/Sylius/Behat/Context/Ui/Admin/LoginContext.php @@ -139,4 +139,12 @@ final class LoginContext implements Context $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 0000000000..544704d13d --- /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 d6430fe52e..f177a0db9c 100644 --- a/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php +++ b/src/Sylius/Behat/Context/Ui/Shop/AccountContext.php @@ -255,9 +255,10 @@ final class AccountContext implements Context } /** + * @Given I am browsing my orders * @When I browse my orders */ - public function iBrowseMyOrders() + public function iBrowseMyOrders(): void { $this->orderIndexPage->open(); } @@ -523,4 +524,20 @@ final class AccountContext implements Context 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 0000000000..3743fa94fc --- /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 0000000000..4f533b13d5 --- /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 659d57b131..36d55417c4 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 34f08b62f9..12b5861363 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 2b2f32caa1..0a3844c97f 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 0000000000..c85a01fbff --- /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 8c074404ec..3ae6fdbe3c 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 0000000000..f67decf3f5 --- /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 afc0a94f1f..0fe5aaf2b3 100644 --- a/src/Sylius/Bundle/ApiBundle/composer.json +++ b/src/Sylius/Bundle/ApiBundle/composer.json @@ -26,6 +26,7 @@ "php": "^8.0", "doctrine/dbal": "^2.7", "api-platform/core": "^2.6", + "enshrined/svg-sanitize": "^0.15.4", "lexik/jwt-authentication-bundle": "^2.6", "sylius/core-bundle": "^1.7", "symfony/messenger": "^5.4" diff --git a/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php b/src/Sylius/Bundle/CoreBundle/EventListener/XFrameOptionsSubscriber.php new file mode 100644 index 0000000000..8e4727682e --- /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 5b90aef2ff..f28993ae14 100644 --- a/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/CoreBundle/Resources/config/services/listeners.xml @@ -82,6 +82,10 @@ + + + + diff --git a/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php b/src/Sylius/Bundle/ShopBundle/EventListener/ShopCustomerAccountSubSectionCacheControlSubscriber.php new file mode 100644 index 0000000000..803d481c9f --- /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 a7ca729897..84f6c97a7a 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 6b254d25f2..c0b6413a4e 100644 --- a/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml +++ b/src/Sylius/Bundle/ShopBundle/Resources/config/services/listeners.xml @@ -32,6 +32,11 @@ + + + + + diff --git a/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php b/src/Sylius/Bundle/ShopBundle/SectionResolver/ShopCustomerAccountSubSection.php new file mode 100644 index 0000000000..c99afa65ec --- /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 0000000000..61fc6e72b8 --- /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 ff71023690..1d687b94d7 100644 --- a/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php +++ b/src/Sylius/Bundle/ShopBundle/spec/SectionResolver/ShopUriBasedSectionResolverSpec.php @@ -15,6 +15,7 @@ namespace spec\Sylius\Bundle\ShopBundle\SectionResolver; 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 @@ final class ShopUriBasedSectionResolverSpec extends ObjectBehavior $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 @@ final class ShopUriBasedSectionResolverSpec extends ObjectBehavior $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 2a92bb7610..8fce537d29 100644 --- a/src/Sylius/Component/Core/Uploader/ImageUploader.php +++ b/src/Sylius/Component/Core/Uploader/ImageUploader.php @@ -13,6 +13,7 @@ declare(strict_types=1); 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,9 +23,15 @@ use Webmozart\Assert\Assert; class ImageUploader implements ImageUploaderInterface { + private const MIME_SVG_XML = 'image/svg+xml'; + private const MIME_SVG = 'image/svg'; + /** @var ImagePathGeneratorInterface */ protected $imagePathGenerator; + /** @var Sanitizer */ + protected $sanitizer; + public function __construct( protected Filesystem $filesystem, ?ImagePathGeneratorInterface $imagePathGenerator = null @@ -37,6 +44,7 @@ class ImageUploader implements ImageUploaderInterface } $this->imagePathGenerator = $imagePathGenerator ?? new UploadedImagePathGenerator(); + $this->sanitizer = new Sanitizer(); } public function upload(ImageInterface $image): void @@ -45,11 +53,13 @@ class ImageUploader implements ImageUploaderInterface 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()); } @@ -60,10 +70,7 @@ class ImageUploader implements ImageUploaderInterface $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 @@ -75,6 +82,15 @@ class ImageUploader implements ImageUploaderInterface 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 96dd9971f5..8ee9f35004 100644 --- a/src/Sylius/Component/Core/composer.json +++ b/src/Sylius/Component/Core/composer.json @@ -27,6 +27,7 @@ ], "require": { "php": "^8.0", + "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 c4b0cf573a..764abe9919 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 0000000000..1011230418 --- /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 0000000000..ae9bd10c94 --- /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 0000000000..c3ad2f1da8 --- /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 0000000000..3bc4cecef1 --- /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 0000000000..08ecd246d4 --- /dev/null +++ b/tests/Resources/xss.svg @@ -0,0 +1,9 @@ + + + + + + +