tonyair/silverstripe-cms
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-cms synced 2024-10-22 08:05:56 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #2798 from creative-commoners/pulls/4.11/cve-2022-37421

Browse Source 
Sanitise ExtraMeta field for XSS
This commit is contained in:
Guy Sartorelli 2022-11-21 12:57:59 +13:00 committed by GitHub
commit 8526067c73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 82 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
code/Model/SiteTree.php
View File

@ -69,6 +69,7 @@ use SilverStripe\Versioned\RecursivePublishable;
use SilverStripe\Versioned\Versioned; use SilverStripe\Versioned\Versioned;
use SilverStripe\View\ArrayData; use SilverStripe\View\ArrayData;
use SilverStripe\View\HTML; use SilverStripe\View\HTML;
use SilverStripe\View\Parsers\HTMLValue;
use SilverStripe\View\Parsers\ShortcodeParser; use SilverStripe\View\Parsers\ShortcodeParser;
use SilverStripe\View\Parsers\URLSegmentFilter; use SilverStripe\View\Parsers\URLSegmentFilter;
use SilverStripe\View\Shortcodes\EmbedShortcodeProvider; use SilverStripe\View\Shortcodes\EmbedShortcodeProvider;
@ -1684,6 +1685,8 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
$this->setNextWriteWithoutVersion(true); $this->setNextWriteWithoutVersion(true);
} }
$this->sanitiseExtraMeta();
// Flush cached [embed] shortcodes // Flush cached [embed] shortcodes
// Flush on both DRAFT and LIVE because VersionedCacheAdapter has separate caches for both // Flush on both DRAFT and LIVE because VersionedCacheAdapter has separate caches for both
// Clear both caches at once for the scenario where a CMS-author updates a remote resource // Clear both caches at once for the scenario where a CMS-author updates a remote resource
@ -1703,6 +1706,27 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
} }
} }
private function sanitiseExtraMeta(): void
{
$htmlValue = HTMLValue::create($this->ExtraMeta);
/** @var DOMElement $el */
foreach ($htmlValue->query('//*') as $el) {
/** @var DOMAttr $attr */
$attributes = $el->attributes;
for ($i = count($attributes) - 1; $i >= 0; $i--) {
$attr = $attributes->item($i);
// remove any attribute starting with 'on' e.g. onclick
// and remove the accesskey attribute
if (substr($attr->name, 0, 2) === 'on' ||
$attr->name === 'accesskey'
) {
$el->removeAttributeNode($attr);
}
}
}
$this->ExtraMeta = $htmlValue->getContent();
}
/** /**
* Trigger synchronisation of link tracking * Trigger synchronisation of link tracking
* *
@ -1797,6 +1821,16 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi
); );
} }
// Ensure ExtraMeta can be turned into valid HTML
if ($this->ExtraMeta && !HTMLValue::create($this->ExtraMeta)->getContent()) {
$result->addError(
_t(
'SilverStripe\\CMS\\Model\\SiteTree.InvalidExtraMeta',
'Custom Meta Tags does not contain valid HTML',
)
);
}
return $result; return $result;
} }

1
lang/en.yml
View File

@ -208,6 +208,7 @@ en:
HTMLEDITORTITLE: Content HTMLEDITORTITLE: Content
INHERIT: 'Inherit from parent page' INHERIT: 'Inherit from parent page'
INHERITSITECONFIG: 'Inherit from site access settings' INHERITSITECONFIG: 'Inherit from site access settings'
InvalidExtraMeta: 'Custom Meta Tags does not contain valid HTML'
LASTPUBLISHED: 'Last published' LASTPUBLISHED: 'Last published'
LASTSAVED: 'Last saved' LASTSAVED: 'Last saved'
LASTUPDATED: 'Last Updated' LASTUPDATED: 'Last Updated'

47
tests/php/Model/SiteTreeTest.php
View File

@ -1986,4 +1986,51 @@ class SiteTreeTest extends SapphireTest
); );
// END ARCHIVED // END ARCHIVED
} }
/**
* @dataProvider provideSanitiseExtraMeta
*/
public function testSanitiseExtraMeta(string $extraMeta, string $expected, string $message): void
{
$siteTree = new SiteTree();
$siteTree->ExtraMeta = $extraMeta;
$siteTree->write();
$this->assertSame($expected, $siteTree->ExtraMeta, $message);
}
public function provideSanitiseExtraMeta(): array
{
return [
[
'<link rel="canonical" accesskey="X" sometrigger="alert(1)" />',
'<link rel="canonical" sometrigger="alert(1)">',
'accesskey attribute is removed'
],
[
'<link rel="canonical" onclick="alert(1)" /><meta name="x" onerror="alert(0)">',
'<link rel="canonical"><meta name="x">',
'Attributes starting with "on" are removed'
],
[
'<link rel="canonical" onclick=alert(1) /><meta name="x" onerror=\'alert(0)\'>',
'<link rel="canonical"><meta name="x">',
'Attributes with different quote styles are removed'
],
[
'<link rel="canonical" ONCLICK=alert(1) /><meta name="x" oNeRrOr=\'alert(0)\'>',
'<link rel="canonical"><meta name="x">',
'Mixed case attributes are removed'
],
[
'<link rel="canonical" accesskey="X" onclick="alert(1)" name="x" />',
'<link rel="canonical" name="x">',
'Multiple attributes are removed'
],
[
'<link rel="canonical" href="valid" ;;// somethingdodgy < onmouseover=alert(1)',
'<link rel="canonical" href="valid" somethingdodgy="">',
'Invalid HTML is converted to valid HTML and parsed'
],
];
}
} }