Bartosz Dziewoński has uploaded a new change for review. https://gerrit.wikimedia.org/r/314349
Change subject: UploadBase: Permit SVG files with broken namespace definition (Inkscape bug) ...................................................................... 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 <https://commons.wikimedia.org/w/?curid=16495597>. * 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). Bug: T144827 Change-Id: I0eb9766cab86a58d729f10033c64f57d2076d917 --- M includes/upload/UploadBase.php A tests/phpunit/data/upload/buggynamespace-bad.svg A tests/phpunit/data/upload/buggynamespace-okay.svg A tests/phpunit/data/upload/buggynamespace-okay2.svg A tests/phpunit/data/upload/buggynamespace-original.svg M tests/phpunit/includes/upload/UploadBaseTest.php 6 files changed, 200 insertions(+), 1 deletion(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/49/314349/1 diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 91b4133..3422632 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1440,7 +1440,11 @@ '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 0000000..974fac0 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-bad.svg @@ -0,0 +1,24 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Generator: Adobe Illustrator 13.0.0, SVG Export Plug-In . SVG Version: 6.00 Build 14948) --> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd";> +<svg version="1.1" + id="svg2" xmlns:x="&ns_extend;" xmlns:i="&ns_ai;" xmlns:graph="&ns_graphs;" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; xmlns:dc="http://purl.org/dc/elements/1.1/"; xmlns:svg="http://www.w3.org/2000/svg"; xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; xmlns:cc="http://creativecommons.org/ns#"; xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; inkscape:output_extension="org.inkscape.output.svg.inkscape" sodipodi:version="0.32" sodipodi:docname="India_location_map.svg" inkscape:version="0.46" + xmlns="http://www.w3.org/2000/svg"; xmlns:xlink="http://www.w3.org/1999/xlink"; x="0px" y="0px" width="1500px" + height="1614.844px" viewBox="0 0 1500 1614.844" enable-background="new 0 0 1500 1614.844" xml:space="preserve"> +<switch> + <foreignObject requiredExtensions="&ns_ai;" x="0" y="0" width="1" height="1"> + <i:pgfRef xlink:href="#adobe_illustrator_pgf"> + </i:pgfRef> + </foreignObject> + <g i:extraneous="self"> + <circle cx="750" cy="750" r="500" /> + </g> +</switch> +<i:pgf id="adobe_illustrator_pgf"> + <![CDATA[ + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + ]]> +</i:pgf> +</svg> diff --git a/tests/phpunit/data/upload/buggynamespace-okay.svg b/tests/phpunit/data/upload/buggynamespace-okay.svg new file mode 100644 index 0000000..4a5c6aa --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-okay.svg @@ -0,0 +1,52 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> +<!-- Created with Inkscape (http://www.inkscape.org/) --> + +<svg + xmlns:i="&ns_ai;" + xmlns:dc="http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:xlink="http://www.w3.org/1999/xlink"; + version="1.1" + width="1500" + height="1614.844" + viewBox="0 0 1500 1614.844" + id="svg2" + xml:space="preserve"><metadata + id="metadata15"><rdf:RDF><cc:Work + rdf:about=""><dc:format>image/svg+xml</dc:format><dc:type + rdf:resource="http://purl.org/dc/dcmitype/StillImage"; /><dc:title></dc:title></cc:Work></rdf:RDF></metadata><defs + id="defs13" /> +<switch + id="switch3"> + <foreignObject + id="foreignObject5" + height="1" + width="1" + y="0" + x="0" + requiredExtensions="http://ns.adobe.com/AdobeIllustrator/10.0/";> + <i:pgfRef + xlink:href="#adobe_illustrator_pgf"> + </i:pgfRef> + </foreignObject> + <g + id="g7"> + <circle + cx="750" + cy="750" + r="500" + id="circle9" /> + </g> +</switch> +<i:pgf + id="adobe_illustrator_pgf"> + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + +</i:pgf> +</svg> \ 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 0000000..fe42310 --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-okay2.svg @@ -0,0 +1,52 @@ +<?xml version="1.0" encoding="UTF-8" standalone="no"?> +<!-- Created with Inkscape (http://www.inkscape.org/) --> + +<svg + xmlns:i="&#38;ns_ai;" + xmlns:dc="http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:xlink="http://www.w3.org/1999/xlink"; + version="1.1" + width="1500" + height="1614.844" + viewBox="0 0 1500 1614.844" + id="svg2" + xml:space="preserve"><metadata + id="metadata15"><rdf:RDF><cc:Work + rdf:about=""><dc:format>image/svg+xml</dc:format><dc:type + rdf:resource="http://purl.org/dc/dcmitype/StillImage"; /><dc:title></dc:title></cc:Work></rdf:RDF></metadata><defs + id="defs13" /> +<switch + id="switch3"> + <foreignObject + requiredExtensions="http://ns.adobe.com/AdobeIllustrator/10.0/"; + x="0" + y="0" + width="1" + height="1" + id="foreignObject5"> + <i:pgfRef + xlink:href="#adobe_illustrator_pgf"> + </i:pgfRef> + </foreignObject> + <g + id="g7"> + <circle + cx="750" + cy="750" + r="500" + id="circle9" /> + </g> +</switch> +<i:pgf + id="adobe_illustrator_pgf"> + + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + +</i:pgf> +</svg> \ 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 0000000..c61c91c --- /dev/null +++ b/tests/phpunit/data/upload/buggynamespace-original.svg @@ -0,0 +1,33 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Generator: Adobe Illustrator 13.0.0, SVG Export Plug-In . SVG Version: 6.00 Build 14948) --> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"; [ + <!ENTITY ns_extend "http://ns.adobe.com/Extensibility/1.0/";> + <!ENTITY ns_ai "http://ns.adobe.com/AdobeIllustrator/10.0/";> + <!ENTITY ns_graphs "http://ns.adobe.com/Graphs/1.0/";> + <!ENTITY ns_vars "http://ns.adobe.com/Variables/1.0/";> + <!ENTITY ns_imrep "http://ns.adobe.com/ImageReplacement/1.0/";> + <!ENTITY ns_sfw "http://ns.adobe.com/SaveForWeb/1.0/";> + <!ENTITY ns_custom "http://ns.adobe.com/GenericCustomNamespace/1.0/";> + <!ENTITY ns_adobe_xpath "http://ns.adobe.com/XPath/1.0/";> +]> +<svg version="1.1" + id="svg2" xmlns:x="&ns_extend;" xmlns:i="&ns_ai;" xmlns:graph="&ns_graphs;" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; xmlns:dc="http://purl.org/dc/elements/1.1/"; xmlns:svg="http://www.w3.org/2000/svg"; xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; xmlns:cc="http://creativecommons.org/ns#"; xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; inkscape:output_extension="org.inkscape.output.svg.inkscape" sodipodi:version="0.32" sodipodi:docname="India_location_map.svg" inkscape:version="0.46" + xmlns="http://www.w3.org/2000/svg"; xmlns:xlink="http://www.w3.org/1999/xlink"; x="0px" y="0px" width="1500px" + height="1614.844px" viewBox="0 0 1500 1614.844" enable-background="new 0 0 1500 1614.844" xml:space="preserve"> +<switch> + <foreignObject requiredExtensions="&ns_ai;" x="0" y="0" width="1" height="1"> + <i:pgfRef xlink:href="#adobe_illustrator_pgf"> + </i:pgfRef> + </foreignObject> + <g i:extraneous="self"> + <circle cx="750" cy="750" r="500" /> + </g> +</switch> +<i:pgf id="adobe_illustrator_pgf"> + <![CDATA[ + eJzsvWl3XjXyL3pf91r9HZ50MyQkfrw1awcIZCBAYyAQaEIzBMd+krjx1LYDzf/F+exXNUml/Qwx +d1/++qv/+P2XX3/z1fc//9mvf/jyH796+/Lbc2Z9+eNXvzn/9Pbr77/64cfvvv/q7Ye//+53hNBH ++sFf/MXf/Nv/5ec/+38A7g2BFw== + ]]> +</i:pgf> +</svg> diff --git a/tests/phpunit/includes/upload/UploadBaseTest.php b/tests/phpunit/includes/upload/UploadBaseTest.php index 3debe6e..1c19665 100644 --- a/tests/phpunit/includes/upload/UploadBaseTest.php +++ b/tests/phpunit/includes/upload/UploadBaseTest.php @@ -398,6 +398,40 @@ } /** + * @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 ); + $this->assertSame( $expected, $result, $message ); + } + + public static function provideDetectScriptInSvg() { + 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' + ], + ]; + } + + /** * @dataProvider provideCheckXMLEncodingMissmatch */ public function testCheckXMLEncodingMissmatch( $fileContents, $evil ) { -- To view, visit https://gerrit.wikimedia.org/r/314349 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0eb9766cab86a58d729f10033c64f57d2076d917 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
