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 potential XSS in the Checkout, Address Book and Admin Panel #16241

Merged
merged 16 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@address_book
Feature: Preventing a potential XSS attack during updating the address
In order to keep my information safe
As a Customer
I want to be protected against the potential XSS attacks

Background:
Given the store operates on a single channel in "United States"
And I am a logged in customer
And I have an address "Lucifer Morningstar", "Seaside Fwy", "90802", "Los Angeles", "United States", "Arkansas" in my address book
And this address has province '<img """><script>alert("XSS")</script>">'

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack during updating the address
When I want to edit the address of "Lucifer Morningstar"
Then I should be able to update it without unexpected alert
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@checkout
Feature: Preventing a potential XSS attack during updating the address in the checkout
In order to keep my information safe
As a Visitor
I want to be protected against the potential XSS attacks

Background:
Given the store operates on a single channel in "United States"
And the store has a product "PHP T-Shirt" priced at "$19.99"
And the store ships everywhere for Free
And I have product "PHP T-Shirt" in the cart
And I am at the checkout addressing step

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack during updating the address in the checkout
When I specify the email as "john.doe@example.com"
And I specify the billing address as "Ankh Morpork", "Frost Alley", "90210", "United States" for "Jon Doe"
And I specify the province name manually as '<img """><script>alert("XSS")</script>">' for billing address
And I complete the addressing step
And I decide to change my address
Then I should be able to update the address without unexpected alert
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
@managing_products
Feature: Preventing a potential XSS attack while adding a new product
In order to keep my information safe
As an Administrator
I want to be protected against the potential XSS attacks

Background:
Given the store operates on a single channel in "United States"
And the store has "<script>alert('xss')</script>" taxonomy
And the store has "No XSS" taxonomy
And I am logged in as an administrator

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack while adding new product
When I want to create a new simple product
Then I should be able to name it "No XSS" in "English (United States)"

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack while choosing main taxon for a new product
When I want to create a new simple product
Then I should be able to choose main taxon "No XSS"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@managing_products
Feature: Preventing a potential XSS attack while selecting similar product
In order to keep my information safe
As an Administrator
I want to be protected against the potential XSS attacks

Background:
Given the store operates on a single channel in "United States"
And the store has a product association type "Accessories"
And the store has "<script>alert('xss')</script>" and "LG headphones" products
And I am logged in as an administrator

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack while editing product
When I want to create a new simple product
Then I should be able to associate as "Accessories" the "LG headphones" product
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@managing_taxons
Feature: Preventing a potential XSS attack while adding a new taxon
In order to keep my information safe
As an Administrator
I want to be protected against the potential XSS attacks

Background:
Given the store operates on a single channel in "United States"
And the store has "Category" taxonomy
And the store has "<script>alert('xss')</script>" taxonomy
And I am logged in as an administrator

@ui @javascript @no-api
Scenario: Preventing a potential XSS attack while adding new taxon
When I want to create a new taxon
Then I should be able to change its parent taxon to "Category"
13 changes: 13 additions & 0 deletions src/Sylius/Behat/Context/Setup/AddressContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@ public function iHaveAnAddressInAddressBook(ShopUserInterface $user, AddressInte
$customer = $user->getCustomer();

$this->addAddressToCustomer($customer, $address);

$this->sharedStorage->set('address', $address);
}

/**
* @Given this address has province :province
*/
public function thisAddressHasProvince(string $provinceName): void
{
$address = $this->sharedStorage->get('address');
$address->setProvinceName($provinceName);

$this->customerManager->flush();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public function iSpecifyItsCodeAs($code = null)
* @When I do not name it
* @When I name it :name in :language
* @When I rename it to :name in :language
* @When I should be able to name it :name in :language
*/
public function iRenameItToIn(?string $name = null, ?string $language = null): void
{
Expand Down Expand Up @@ -747,6 +748,7 @@ public function theOptionFieldShouldBeDisabled()

/**
* @When /^I choose main (taxon "[^"]+")$/
* @Then /^I should be able to choose main (taxon "[^"]+")$/
*/
public function iChooseMainTaxon(TaxonInterface $taxon)
{
Expand Down Expand Up @@ -820,6 +822,7 @@ public function iAttachImageWithType($path, $type = null)
/**
* @When I associate as :productAssociationType the :productName product
* @When I associate as :productAssociationType the :firstProductName and :secondProductName products
* @Then I should be able to associate as :productAssociationType the :productName product
*/
public function iAssociateProductsAsProductAssociation(
ProductAssociationTypeInterface $productAssociationType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public function iDescribeItAs($description, $language)
/**
* @Given /^I set its (parent taxon to "[^"]+")$/
* @Given /^I change its (parent taxon to "[^"]+")$/
* @Then /^I should be able to change its (parent taxon to "[^"]+")$/
*/
public function iChangeItsParentTaxonTo(TaxonInterface $taxon)
{
Expand Down
11 changes: 10 additions & 1 deletion src/Sylius/Behat/Context/Ui/Shop/AddressBookContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ public function __construct(

/**
* @Given I am editing the address of :fullName
* @When I want to edit the address of :fullName
*/
public function iEditAddressOf($fullName)
public function iEditAddressOf(string $fullName): void
{
$this->sharedStorage->set('full_name', $fullName);

Expand Down Expand Up @@ -350,6 +351,14 @@ public function addressShouldBeMarkedAsMyDefaultAddress(AddressInterface $addres
Assert::same($actualFullName, $expectedFullName);
}

/**
* @Then I should be able to update it without unexpected alert
*/
public function iShouldBeAbleToUpdateItWithoutUnexpectedAlert(): void
{
$this->addressBookUpdatePage->waitForFormToStopLoading();
}

/**
* @param string $fullName
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,14 @@ public function shouldHaveCountriesToChooseFrom(string ...$countries): void
Assert::same($availableBillingCountries, $countries);
}

/**
* @Then I should be able to update the address without unexpected alert
*/
public function iShouldBeAbleToUpdateTheAddressWithoutUnexpectedAlert(): void
{
$this->addressPage->waitForFormToStopLoading();
}

/**
* @return AddressInterface
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public function selectCountry(string $name): void
JQueryHelper::waitForFormToStopLoading($this->getDocument());
}

public function waitForFormToStopLoading(): void
{
JQueryHelper::waitForFormToStopLoading($this->getDocument());
}

public function saveChanges(): void
{
JQueryHelper::waitForFormToStopLoading($this->getDocument());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,7 @@ public function selectProvince(string $name): void;

public function selectCountry(string $name): void;

public function waitForFormToStopLoading(): void;

public function saveChanges(): void;
}
5 changes: 5 additions & 0 deletions src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@ public function getAvailableBillingCountries(): array
return $this->getOptionsFromSelect($this->getElement('billing_country'));
}

public function waitForFormToStopLoading(): void
{
JQueryHelper::waitForFormToStopLoading($this->getDocument());
}

protected function getDefinedElements(): array
{
return array_merge(parent::getDefinedElements(), [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,6 @@ public function getAvailableBillingCountries(): array;
public function isDifferentShippingAddressChecked(): bool;

public function isShippingAddressVisible(): bool;

public function waitForFormToStopLoading(): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import 'semantic-ui-css/components/api';
import 'semantic-ui-css/components/checkbox';
import $ from 'jquery';
import { sanitizeInput } from "sylius/ui/sylius-sanitizer";

const createRootContainer = function createRootContainer() {
return $('<div class="ui list"></div>');
Expand Down Expand Up @@ -81,7 +82,7 @@ $.fn.extend({
onSuccess(response) {
response.forEach((leafNode) => {
leafContainerElement.append((
createLeafFunc(leafNode.name, leafNode.code, leafNode.hasChildren, multiple, leafNode.level)
createLeafFunc(sanitizeInput(leafNode.name), sanitizeInput(leafNode.code), leafNode.hasChildren, multiple, leafNode.level)
));
});
content.append(leafContainerElement);
Expand Down Expand Up @@ -169,7 +170,7 @@ $.fn.extend({
const rootContainer = createRootContainer();
response.forEach((rootNode) => {
rootContainer.append((
createLeaf(rootNode.name, rootNode.code, rootNode.hasChildren, multiple, rootNode.level)
createLeaf(sanitizeInput(rootNode.name), sanitizeInput(rootNode.code), rootNode.hasChildren, multiple, rootNode.level)
));
});
tree.append(rootContainer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
*/

import $ from 'jquery';
import { sanitizeInput } from 'sylius/ui/sylius-sanitizer';

const getProvinceInputValue = function getProvinceInputValue(valueSelector) {
return valueSelector == undefined ? '' : `value="${valueSelector}"`;
return valueSelector == undefined ? '' : `value="${sanitizeInput(valueSelector)}"`;
};

$.fn.extend({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import 'semantic-ui-css/components/dropdown';
import $ from 'jquery';
import { sanitizeInput } from "./sylius-sanitizer";

$.fn.extend({
autoComplete() {
Expand Down Expand Up @@ -37,8 +38,8 @@ $.fn.extend({
},
onResponse(response) {
let results = response.map(item => ({
name: item[choiceName],
value: item[choiceValue],
name: sanitizeInput(item[choiceName]),
value: sanitizeInput(item[choiceValue]),
}));

if (!element.hasClass('multiple')) {
Expand Down Expand Up @@ -72,7 +73,7 @@ $.fn.extend({
onSuccess(response) {
response.forEach((item) => {
menuElement.append((
$(`<div class="item" data-value="${item[choiceValue]}">${item[choiceName]}</div>`)
$(`<div class="item" data-value="${item[choiceValue]}">${sanitizeInput(item[choiceName])}</div>`)
));
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import 'semantic-ui-css/components/dropdown';
import $ from 'jquery';
import { sanitizeInput } from "./sylius-sanitizer";

$.fn.extend({
productAutoComplete() {
Expand Down Expand Up @@ -38,8 +39,8 @@ $.fn.extend({
return {
success: true,
results: response._embedded.items.map(item => ({
name: item.name,
value: item.code,
name: sanitizeInput(item.name),
value: sanitizeInput(item.code),
})),
};
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function sanitizeInput(input) {
const div = document.createElement('div');
div.textContent = input;
return div.innerHTML; // Converts text content to plain HTML, stripping any scripts
}