Skip to content

Commit

Permalink
Update underlying library to fix XSS issue in href values
Browse files Browse the repository at this point in the history
Fixes #9
  • Loading branch information
darylldoyle committed Nov 7, 2019
1 parent aa274a6 commit e9e7d60
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 35 deletions.
10 changes: 5 additions & 5 deletions lib/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions lib/vendor/composer/installed.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
[
{
"name": "enshrined/svg-sanitize",
"version": "0.12.0",
"version_normalized": "0.12.0.0",
"version": "0.13.0",
"version_normalized": "0.13.0.0",
"source": {
"type": "git",
"url": "https://github.com/darylldoyle/svg-sanitizer.git",
"reference": "51ca4b713f3706d6b27769c6296bbc0c28a5bbd0"
"reference": "4cf8d0f61edf9f00b84e162fc229176a362da247"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/51ca4b713f3706d6b27769c6296bbc0c28a5bbd0",
"reference": "51ca4b713f3706d6b27769c6296bbc0c28a5bbd0",
"url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/4cf8d0f61edf9f00b84e162fc229176a362da247",
"reference": "4cf8d0f61edf9f00b84e162fc229176a362da247",
"shasum": ""
},
"require": {
Expand All @@ -22,7 +22,7 @@
"codeclimate/php-test-reporter": "^0.1.2",
"phpunit/phpunit": "^6"
},
"time": "2019-10-21T22:39:08+00:00",
"time": "2019-11-07T09:16:31+00:00",
"type": "library",
"installation-source": "dist",
"autoload": {
Expand Down
74 changes: 52 additions & 22 deletions lib/vendor/enshrined/svg-sanitize/src/Sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
class Sanitizer
{

/**
* Regex to catch script and data values in attributes
*/
const SCRIPT_REGEX = '/(?:\w+script|data)(?:\s)?:/xi';

/**
* @var \DOMDocument
*/
Expand Down Expand Up @@ -372,22 +367,12 @@ protected function cleanAttributesOnWhitelist(\DOMElement $element)
protected function cleanXlinkHrefs(\DOMElement $element)
{
$xlinks = $element->getAttributeNS('http://www.w3.org/1999/xlink', 'href');
if (preg_match(self::SCRIPT_REGEX, $xlinks) === 1) {
if (!in_array(substr($xlinks, 0, 14), array(
'data:image/png', // PNG
'data:image/gif', // GIF
'data:image/jpg', // JPG
'data:image/jpe', // JPEG
'data:image/pjp', // PJPEG
))) {
$element->removeAttributeNS( 'http://www.w3.org/1999/xlink', 'href' );
$this->xmlIssues[] = array(
'message' => 'Suspicious attribute \'href\'',
'line' => $element->getLineNo(),
);


}
if (false === $this->isHrefSafeValue($xlinks)) {
$element->removeAttributeNS( 'http://www.w3.org/1999/xlink', 'href' );
$this->xmlIssues[] = array(
'message' => 'Suspicious attribute \'href\'',
'line' => $element->getLineNo(),
);
}
}

Expand All @@ -399,7 +384,7 @@ protected function cleanXlinkHrefs(\DOMElement $element)
protected function cleanHrefs(\DOMElement $element)
{
$href = $element->getAttribute('href');
if (preg_match(self::SCRIPT_REGEX, $href) === 1) {
if (false === $this->isHrefSafeValue($href)) {
$element->removeAttribute('href');
$this->xmlIssues[] = array(
'message' => 'Suspicious attribute \'href\'',
Expand All @@ -408,6 +393,51 @@ protected function cleanHrefs(\DOMElement $element)
}
}

/**
* Only allow whitelisted starts to be within the href.
*
* This will stop scripts etc from being passed through, with or without attempting to hide bypasses.
* This stops the need for us to use a complicated script regex.
*
* @param $value
* @return bool
*/
protected function isHrefSafeValue($value) {

// Allow fragment identifiers.
if ('#' === substr($value, 0, 1)) {
return true;
}

// Allow relative URIs.
if ('/' === substr($value, 0, 1)) {
return true;
}

// Allow HTTPS domains.
if ('https://' === substr($value, 0, 8)) {
return true;
}

// Allow HTTP domains.
if ('http://' === substr($value, 0, 7)) {
return true;
}

// Allow known data URIs.
if (in_array(substr($value, 0, 14), array(
'data:image/png', // PNG
'data:image/gif', // GIF
'data:image/jpg', // JPG
'data:image/jpe', // JPEG
'data:image/pjp', // PJPEG
))) {
return true;
}

return false;
}

/**
* Removes non-printable ASCII characters from string & trims it
*
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions lib/vendor/enshrined/svg-sanitize/tests/data/useClean.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 4 additions & 1 deletion readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Tags: svg, sanitize, upload, sanitise, security, svg upload, image, vector, file
Requires at least: 4.0
Tested up to: 5.2.2
Requires PHP: 5.6
Stable tag: 1.9.5
Stable tag: 1.9.6
License: GPLv2 or later
License URI: http://www.gnu.org/licenses/gpl-2.0.html

Expand Down Expand Up @@ -72,6 +72,9 @@ They take one argument that must be returned. See below for examples:

== Changelog ==

= 1.9.6 =
* Underlying library update that fixes a security issue

= 1.9.5 =
* Underlying library update that fixes some security issues

Expand Down
2 changes: 1 addition & 1 deletion safe-svg.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Plugin Name: Safe SVG
Plugin URI: https://wpsvg.com/
Description: Allows SVG uploads into WordPress and sanitizes the SVG before saving it
Version: 1.9.5
Version: 1.9.6
Author: Daryll Doyle
Author URI: http://enshrined.co.uk
Text Domain: safe-svg
Expand Down

0 comments on commit e9e7d60

Please sign in to comment.