Skip to content

Commit

Permalink
Fixes possible xss via version name, AS-339
Browse files Browse the repository at this point in the history
  • Loading branch information
AltamashShaikh committed Nov 26, 2024
1 parent 3a2e04c commit 8fd297b
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 35 deletions.
5 changes: 5 additions & 0 deletions API.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
use Piwik\Plugins\TagManager\Template\Variable\VariablesProvider;
use Exception;
use Piwik\UrlHelper;
use Piwik\Validators\BaseValidator;
use Piwik\Validators\CharacterLength;
use Piwik\Validators\NotEmpty;

/**
* API for plugin Tag Manager.
Expand Down Expand Up @@ -1167,6 +1170,7 @@ public function createContainerVersion($idSite, $idContainer, $name, $descriptio
$this->accessValidator->checkUseCustomTemplatesCapability($idSite);
}
$this->containers->checkContainerExists($idSite, $idContainer);
BaseValidator::check(Piwik::translate('TagManager_VersionName'), $name, [new NotEmpty(), new CharacterLength(1, 30)]);

if (empty($idContainerVersion)) {
$idContainerVersion = $this->getContainerDraftVersion($idSite, $idContainer);
Expand Down Expand Up @@ -1196,6 +1200,7 @@ public function updateContainerVersion($idSite, $idContainer, $idContainerVersio
if (!Piwik::isUserHasCapability($idSite, PublishLiveContainer::ID) && !Piwik::isUserHasCapability($idSite, PublishLiveContainer::ID)) {
$this->accessValidator->checkUseCustomTemplatesCapability($idSite);
}
BaseValidator::check(Piwik::translate('TagManager_VersionName'), $name, [new NotEmpty(), new CharacterLength(1, 30)]);
$this->containers->checkContainerVersionExists($idSite, $idContainer, $idContainerVersion);

return $this->containers->updateContainerVersion($idSite, $idContainer, $idContainerVersion, $name, $description);
Expand Down
2 changes: 1 addition & 1 deletion tests/Fixtures/TagManagerFixture.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public function setUpContainers()
$this->idContainer1Version3 = $this->api->createContainerVersion($this->idSite2, $this->idContainer1, 'container1_v3', 'Version from draft with tags, triggers and variables');
$this->idContainer1Version4 = $this->api->createContainerVersion($this->idSite2, $this->idContainer1, 'container1_v4_reversioned', 'new version from an older version', $this->idContainer1Version2);

$this->idContainerQuotesVersion1 = $this->api->createContainerVersion($this->idSite2, $this->idContainerQuotes, 'container1_v4_reversioned "Quotes"', 'new version for quotes container');
$this->idContainerQuotesVersion1 = $this->api->createContainerVersion($this->idSite2, $this->idContainerQuotes, 'container1_v4_rev "Quotes"', 'new version for quotes container');

$this->addContainerVariable($this->idSite2, $this->idContainer1, $this->idContainer1DraftVersion, null, 'My Var 3', array('dataLayerName' => 'dataVarName'), false, [], 'My Var 3 description');

Expand Down
19 changes: 19 additions & 0 deletions tests/Integration/APITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,16 @@ public function test_updateContainerVersion_shouldFailWhenContainerVersionDoesNo
$this->api->updateContainerVersion($this->idSite, $this->idContainer, 99999, 'TheName');
}

public function test_updateContainerVersion_shouldThrowExceptionForInvalidNameLength()
{
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Version name: The value contains "48" characters but should contain at most 30 characters.');

$this->setSuperUser();
$idContainerVersion = $this->api->createContainerVersion($this->idSite, $this->idContainer, 'My Name');
$this->api->updateContainerVersion($this->idSite, $this->idContainer, $idContainerVersion, 'My Name very long name should throw an exception', 'TheName');
}

public function test_createContainerVersion_shouldFailWhenNotHavingViewPermissions()
{
$this->expectException(\Piwik\NoAccessException::class);
Expand All @@ -402,6 +412,15 @@ public function test_createContainerVersion_shouldFailWhenContainerVersionDoesNo
$this->api->createContainerVersion($this->idSite, $this->idContainer, 'TheName');
}

public function test_createContainerVersion_shouldThrowExceptionForInvalidNameLength()
{
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Version name: The value contains "48" characters but should contain at most 30 characters.');

$this->setSuperUser();
$this->api->createContainerVersion($this->idSite, $this->idContainer, 'My Name very long name should throw an exception');
}

public function test_deleteContainerVersion_shouldFailWhenNotHavingViewPermissions()
{
$this->expectException(\Piwik\NoAccessException::class);
Expand Down
Loading

0 comments on commit 8fd297b

Please sign in to comment.