Page MenuHomePhabricator

Permit url("data:image/svg+xml,...") in CSS Sanitizer if no external access
Open, LowestPublic

Description

I just learned that

background-image: url("data:image/svg+xml,%3Csvg ...")

apparently does not work.

I do understand that usage of url() is limited to well known http domains, like the own site and Commons.

I guess data:image/svg+xml is considered to be dangerous, since an SVG document may contain references to really external URL (embedded beacon PNG etc.).

However, could SVG data be decoded by CSS Sanitizer and source inspected whether containing undesired syntax, or free of any harmful components and get released as sanitized-css?

Event Timeline

Anomie triaged this task as Lowest priority.Jan 14 2019, 3:25 PM
Anomie edited projects, added TemplateStyles; removed css-sanitizer.
Anomie added a subscriber: Anomie.

css-sanitizer by default allows all URLs. It also allows for users of the library to subclass Wikimedia\CSS\Grammar\MatcherFactory and override ->urlstring() and ->url() to provide matchers that filter the allowed URLs.

From your description here, I'm going to guess you're actually complaining about TemplateStyles, which does such an override.

Personally I doubt that adding logic to decode and analyze files (SVG or otherwise) provided as data: URIs would be worth the time and added complexity, when the files could instead be uploaded to Commons and used from there. But I'll leave this open in case someone wants to try writing the necessary code.

As far I know the following procedure should be sufficient, but a real SVG expert might check for further mouse holes:

  1. URL-decode the url() value as inspected string.
  2. Remove any whitespace from test string, even strange stuff. Or just leave ASCII letters and a few special characters <>#/:=.
  3. Downcase the entire thing.
  4. Look for the following strings:
    1. <image
    2. <script
    3. <use (may be permitted as <use href=#template)
    4. href= (but href=# is okay; perhaps also / relative URL). Just to make it bullet proof.
  5. If any of those (or some more) is found, then drop.

I took that from use, image and script. An SVG hero might know other leaks. As far I can see it would be sufficient to blacklist such three elements.

Yes, it is in context of TemplateStyles, for decorative stuff not really needing an independent Commons file to be maintained. SVG may be the only permitted format and would be sufficient.

Besides code complexity, using real SVGs makes review easier and attacks based on misleading images (clickjacking etc) somewhat harder.

This opens another question: Is it actually checked that no SVG on Commons does contain any <image>, <script>, <use>?

This opens another question: Is it actually checked that no SVG on Commons does contain any <image>, <script>, <use>?

Yes, see UploadBase::detectScriptInSvg().

Yeah, thank you, established in July 2009. Did the predecessor check all files before 2009 on every Wiki? Or any monster sleeping in a dark cave?

And checkSvgScriptCallback rules do permit <use>, as far I can see. <image> is limited to href=#... and seems to be safe. Apparently it is permitted to access external beacon by <use xlink:href="http://example.com/beacon.svg">. May be this task is to be turned into Security Issue.

Anyway, since there is an existing SVG source checking PHP available, any CSS url("data:image/svg") is to be run as if it would be an SVG upload.

May be this task is to be turned into Security Issue.

Better to file a new task for that, if you want.

hrefs are filtered in general. See the part with the comment Do not allow relative links, or unsafe url schemas. For <a> tags, only data:, http: and https: and same-document fragment links are allowed. For all other tags, only data: and fragment are allowed. (and then there is some more filtering for data: after that). No idea about old files.

I'm happy to say problems you describe don't exist 🙂. After a day of finding weird security bugs (in other companies) I'm happy to report this works as expected 😎.

In the url context you cannot:

If you open the fiddles with DevTools open, note that only an image with "embed" query is downloaded. Tested with Firefox and Chrome and even IE 11.

So. Backgrounds are safe by design. Please enable this 🙂.