mgeb / mediawiki-drawio-editor

MediaWiki extension that integrates the draw.io flow chart editor.
MIT License
20 stars 21 forks source link

This SVG file contains an illegal namespace "http://www.w3.org/1999/xhtml". #1

Open raarts opened 8 years ago

raarts commented 8 years ago

Add any object, like a rectangle for instance, double-click it, add some text, and save. Results in the above error.

This error is generated in ./includes/upload/UploadBase.php which checks the uploaded svg, and finds this (non-whitelisted) namespace. It refers to this issue: https://phabricator.wikimedia.org/T62771 which in turn points to https://bugzilla.mozilla.org/show_bug.cgi?id=966734. Apparently this particular namespace can result in a persistent xss vulnerability.

I guess this namespace is generated by draw.io. Since you probably are more well-versed in this matter, what's the next step?

EDIT: using png works.

mgeb commented 8 years ago

Your guess is correct. All SVG data is generated by draw.io. On save, the extension receives the data from the draw.io iframe and uploads it to the wiki unaltered. If the wiki decides not to accept the upload for some reason, then that's beyond the control of the plugin.

My initial reaction to your issue was that draw.io must have changed something so that its SVG output now contains these insecure namespaces. Then again, they seem to be about iframes within SVG and why would your test case require iframes at all? Testing confirms that the extension still works in my environment and I cannot reproduce the problem with the steps you provided. So I don't think draw.io changed its output since I've written the plugin. It seems more likely that draw.io produces different output in your environment. Could you provide some information about your setup?

I'm using the extension with MediaWiki 1.25 and tested it against very recent versions of Safari, Firefox and Chrome.

Also, can you reproduce the problem when you use draw.io directly, export an SVG file (containing the SVG data and the draw.io XML) and upload it to your wiki manually?

raarts commented 8 years ago

Right. I created a file directly on draw.io, and tried to upload to my wiki (1.26.2). Same error. I opened a support issue at draw.io. I'll keep this ticket updated.

EDIT: I guess MW stopped accepting it in the newer version I use.

test-svg.zip

mgeb commented 8 years ago

I could not verify it yet, because I haven't had a chance to install 1.26 somewhere, but it seems very likely that this is cause.

I've just looked at the XML produced by draw.io. The offending xhtml namespaces seem to be in the the SVG part of the file. So as far as I can tell, there's no work around, not even storing the draw.io data in a separate file would help.

Thanks for opening a request at draw.io. Keep me posted.

mgeb commented 8 years ago

I've done some more digging. MediaWiki 1.25 already contains the changes you've mentioned in the original post. This means in theory I should not be able to upload a chart generated by draw.io, but in fact my installation does not reject files containing an xhtml namespace. There must be another difference between 1.25 and 1.26 that prevents you from uploading the SVG files.

Anyway, I've had closer look at the SVG files produced by draw.io and to me the use of the xhtml namespace seems legitimate. The text contained in the box is added as both xhtml and pure SVG within a switch tag:

  <g transform="translate(0.5,0.5)">
    <rect x="1" y="1" width="120" height="60" rx="9" ry="9" fill="#000000" stroke="#000000" transform="translate(2,3)" opacity="0.25"/>
    <rect x="1" y="1" width="120" height="60" rx="9" ry="9" fill="url(#mx-gradient-ffcd28-1-ffa500-1-s-0)" stroke="#d79b00" pointer-events="none"/>
    <g transform="translate(39.5,24.5)">
      <switch>
        <foreignObject style="overflow:visible;" pointer-events="all" width="42" height="12" requiredFeatures="http://www.w3.org/TR/SVG11/feature#Extensibility">
          <div xmlns="http://www.w3.org/1999/xhtml" style="display: inline-block; font-size: 12px; font-family: Helvetica; color: rgb(0, 0, 0); line-height: 1.2; vertical-align: top; width: 42px; white-space: nowrap; word-wrap: normal; text-align: center;">
            <div xmlns="http://www.w3.org/1999/xhtml" style="display:inline-block;text-align:inherit;text-decoration:inherit;">asterisk</div>
          </div>
        </foreignObject>
        <text x="21" y="12" fill="#000000" text-anchor="middle" font-size="12px" font-family="Helvetica">asterisk</text>
      </switch>
    </g>

If the user agent supports the foreignObject tag (a.k.a. the Extensibility feature) it will prefer the xhtml version. Otherwise it will fall back to the SVG version (text tag). The former seems to have the advantage of supporting automatic word wrapping which SVG does not seem to be able to do natively.

While I understand that the MediaWiki folks made this restriction to prevent XSS issues with iframes in SVG files, I'm starting to think they might have gone to far by forbidding any xhtml in SVG files. There seem to be good reasons for xhtml in SVG, like the word wrapping and probably more.

floriandotorg commented 8 years ago

As a hacky fix you can add 'http://www.w3.org/1999/xhtml' to $validNamespaces in UploadBase.php

mgeb commented 8 years ago

Correct. So the following workarounds currently exist:

Of course all of them have drawbacks. But I'll probably still mention them in the installation instruction for the time being.

mgeb commented 8 years ago

I still think that MediaWiki is too strict about the xhtml namespace, so I've created an issue at Wikimedia's phabricator to address this:

https://phabricator.wikimedia.org/T138783