From 19d692051f08985363f0f7c02c7ff08b3936ea3d Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Wed, 5 Oct 2016 22:18:39 +0200 Subject: [PATCH] UploadBase: Permit SVG files with broken namespace definition (Inkscape bug) Inkscape mangles namespace definitions created by Adobe Illustrator (apparently it can't parse custom entities or something, maybe just in 'xmlns' attributes). These files are still valid SVG, and not a security issue (although Illustrator probably won't like them), so it's okay to allow them. Added tests with some example files. * buggynamespace-original.svg File generated by Illustrator (edited by hand to reduce filesize). Based on . * buggynamespace-okay.svg The original file, opened and saved in Inkscape (no other changes). * buggynamespace-okay2.svg The original file, opened and saved in Inkscape twice. * buggynamespace-bad.svg The original file, edited by hand to remove custom entities. This is not valid XML and should be rejected (although it's valid when parsed as HTML, and some image viewers might display it). * buggynamespace-evilhtml.svg An SVG file using an entity declared namespace for a namespace we want to ban. Based on buggynamespace-original.svg. Bug: T144827 Change-Id: I0eb9766cab86a58d729f10033c64f57d2076d917 --- includes/upload/UploadBase.php | 6 ++- .../data/upload/buggynamespace-bad.svg | 24 +++++++++ .../data/upload/buggynamespace-evilhtml.svg | 12 +++++ .../data/upload/buggynamespace-okay.svg | 52 +++++++++++++++++++ .../data/upload/buggynamespace-okay2.svg | 52 +++++++++++++++++++ .../data/upload/buggynamespace-original.svg | 33 ++++++++++++ .../includes/upload/UploadBaseTest.php | 47 +++++++++++++++++ 7 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/data/upload/buggynamespace-bad.svg create mode 100644 tests/phpunit/data/upload/buggynamespace-evilhtml.svg create mode 100644 tests/phpunit/data/upload/buggynamespace-okay.svg create mode 100644 tests/phpunit/data/upload/buggynamespace-okay2.svg create mode 100644 tests/phpunit/data/upload/buggynamespace-original.svg diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 91b4133c5d..34226323ad 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1440,7 +1440,11 @@ abstract class UploadBase { 'http://www.w3.org/tr/rec-rdf-syntax/', ]; - if ( !in_array( $namespace, $validNamespaces ) ) { + // Inkscape mangles namespace definitions created by Adobe Illustrator. + // This is nasty but harmless. (T144827) + $isBuggyInkscape = preg_match( '/^&(#38;)*ns_[a-z_]+;$/', $namespace ); + + if ( !( $isBuggyInkscape || in_array( $namespace, $validNamespaces ) ) ) { wfDebug( __METHOD__ . ": Non-svg namespace '$namespace' in uploaded file.\n" ); /** @todo Return a status object to a closure in XmlTypeCheck, for MW1.21+ */ $this->mSVGNSError = $namespace; diff --git a/tests/phpunit/data/upload/buggynamespace-bad.svg b/tests/phpunit/data/upload/buggynamespace-bad.svg new file mode 100644 index 0000000000..974fac0623 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-bad.svg @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + + + diff --git a/tests/phpunit/data/upload/buggynamespace-evilhtml.svg b/tests/phpunit/data/upload/buggynamespace-evilhtml.svg new file mode 100644 index 0000000000..f4be479ed8 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-evilhtml.svg @@ -0,0 +1,12 @@ + + + +]> + +
foo
 + diff --git a/tests/phpunit/data/upload/buggynamespace-okay.svg b/tests/phpunit/data/upload/buggynamespace-okay.svg new file mode 100644 index 0000000000..4a5c6aae23 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-okay.svg @@ -0,0 +1,52 @@ + + + +image/svg+xml + + + + + + + + + + + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + + + \ No newline at end of file diff --git a/tests/phpunit/data/upload/buggynamespace-okay2.svg b/tests/phpunit/data/upload/buggynamespace-okay2.svg new file mode 100644 index 0000000000..fe42310f63 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-okay2.svg @@ -0,0 +1,52 @@ + + + +image/svg+xml + + + + + + + + + + + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + + + \ No newline at end of file diff --git a/tests/phpunit/data/upload/buggynamespace-original.svg b/tests/phpunit/data/upload/buggynamespace-original.svg new file mode 100644 index 0000000000..c61c91cfd8 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-original.svg @@ -0,0 +1,33 @@ + + + + + + + + + + +]> + + + + + + + + + + + + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + + + diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php index 3debe6e198..a44926b208 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -397,6 +397,46 @@ class UploadBaseTest extends MediaWikiTestCase { // @codingStandardsIgnoreEnd } + /** + * @dataProvider provideDetectScriptInSvg + */ + public function testDetectScriptInSvg( $svg, $expected, $message ) { + // This only checks some weird cases, most tests are in testCheckSvgScriptCallback() above + $result = $this->upload->detectScriptInSvg( $svg, false ); + $this->assertSame( $expected, $result, $message ); + } + + public static function provideDetectScriptInSvg() { + global $IP; + return [ + [ + "$IP/tests/phpunit/data/upload/buggynamespace-original.svg", + false, + 'SVG with a weird but valid namespace definition created by Adobe Illustrator' + ], + [ + "$IP/tests/phpunit/data/upload/buggynamespace-okay.svg", + false, + 'SVG with a namespace definition created by Adobe Illustrator and mangled by Inkscape' + ], + [ + "$IP/tests/phpunit/data/upload/buggynamespace-okay2.svg", + false, + 'SVG with a namespace definition created by Adobe Illustrator and mangled by Inkscape (twice)' + ], + [ + "$IP/tests/phpunit/data/upload/buggynamespace-bad.svg", + [ 'uploadscriptednamespace', 'i' ], + 'SVG with a namespace definition using an undefined entity' + ], + [ + "$IP/tests/phpunit/data/upload/buggynamespace-evilhtml.svg", + [ 'uploadscriptednamespace', 'http://www.w3.org/1999/xhtml' ], + 'SVG with an html namespace encoded as an entity' + ], + ]; + } + /** * @dataProvider provideCheckXMLEncodingMissmatch */ @@ -442,4 +482,11 @@ class UploadTestHandler extends UploadBase { ); return [ $check->wellFormed, $check->filterMatch ]; } + + /** + * Same as parent function, but override visibility to 'public'. + */ + public function detectScriptInSvg( $filename, $partial ) { + return parent::detectScriptInSvg( $filename, $partial ); + } } -- 2.20.1