Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
raarts opened this issue May 22, 2016 · 7 comments

Comments

@raarts
Copy link

raarts commented May 22, 2016

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
Copy link
Owner

mgeb commented May 23, 2016

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
Copy link
Author

raarts commented May 23, 2016

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
Copy link
Owner

mgeb commented May 23, 2016

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
Copy link
Owner

mgeb commented May 24, 2016

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
Copy link

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

@mgeb
Copy link
Owner

mgeb commented Jun 27, 2016

Correct. So the following workarounds currently exist:

  • Use MediaWiki 1.25 or older
  • Patch UploadBase.php
  • Use PNG instead of SVG

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

@mgeb
Copy link
Owner

mgeb commented Jun 27, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants