search

npm / npm-explicit-installs

The package that contains the list of packages displayed on the npm home page
5 stars 7 forks source link

how do we support logos in a secure full-featured way? #13

Open bcoe opened 8 years ago

bcoe commented 8 years ago

There's a great conversation here: https://github.com/npm/npm-explicit-installs/pull/12, I'd like to move over to this issue.

At sometime in the future, it would be wonderful to solve the problem of serving logos out of the package.json that fit the following criteria:

mathiasbynens commented 8 years ago

It’d suck to not be able to use SVG for this. @cure53 Is there a secure way to display user-supplied SVGs?

Some quick thoughts:

  1. npmjs.org should include <img src=foo.svg> rather than inlining the SVG or using data URLs, because <img src=foo.svg> never executes scripts embedded in foo.svg.
  2. Of course, browsing to foo.svg directly would execute the scripts, so the SVG files should be hosted on another, dedicated origin (GitHub Pages-style).

→ <img src=https://dedicated-svg-host.example.com/foo.svg>

sindresorhus commented 8 years ago

Is there any CSP rule to disallow script execution in SVG? If not, there definitely should be.

kevinSuttle commented 8 years ago

@bcoe How often are different sizes used?

Also, it seems the script-execution from SVG issue has been pretty heavily documented. This appears to be a task with fairly minimal config.

https://www.w3.org/wiki/SVG_Security

kevinSuttle commented 8 years ago

Eh, maybe not. https://html5sec.org/#svg

What about noreferrer

mathiasbynens commented 8 years ago

Is there any CSP rule to disallow script execution in SVG? If not, there definitely should be.

:+1: No one would be able to use it though, until all older browsers that don’t support this CSP feature aren’t used anymore.

What about noreferrer?

How is this related?

cure53 commented 8 years ago

Hi all, thanks for including us in the discussion.

Before we dive into specific solutions or plans - what means "secure" here? To answer that question, we should first focus on:

In essence, SVG via <img> is safe when safe means XSS. But we need to also make sure that i.e. the host the images reside on is dedicated for that purpose etc etc.

Interested in hearing more!

kevinSuttle commented 8 years ago

@mathias Sorry, meant to link here. I was thinking you could set noreferrer in the svg markup to disable external fetching but I misunderstood how it actually works. https://github.com/w3c/webappsec/issues/139#issuecomment-76741512

kevinSuttle commented 8 years ago

@bcoe Check this out. GitHub has a sanitizer that prevents JS inside of SVG.

https://github.com/jch/html-pipeline/blob/1b5058918eeb0507ac225934cd3e9238f0b94139/lib/html/pipeline/sanitization_filter.rb#L59-L75 via https://olivermak.es/2016/01/svg-in-github-readme/

cure53 commented 8 years ago

@kevinSuttle Has this been tested for security bugs or possible bypasses?

mathiasbynens commented 8 years ago

If anyone manages to bypass it: https://bounty.github.com/submit-a-vulnerability.html

cure53 commented 8 years ago

@mathiasbynens Aye, I didn't realize they use it themselves. Any idea where it can be tested?

kevinSuttle commented 8 years ago

cc @simeonwillbanks

bcoe commented 8 years ago

@cure53 is that whitelist similar to the approach that dompurify uses?

simeonwillbanks commented 8 years ago

@kevinSuttle I wasn't involved with SVG vulnerability testing.

cure53 commented 8 years ago

@bcoe Seems so, would love to see it in action somewhere for a test :)

mathiasbynens commented 8 years ago

@kevinSuttle

GitHub has a sanitizer that prevents JS inside of SVG.

https://github.com/jch/html-pipeline/blob/1b5058918eeb0507ac225934cd3e9238f0b94139/lib/html/pipeline/sanitization_filter.rb#L59-L75 via https://olivermak.es/2016/01/svg-in-github-readme/

The sanitizer you linked to is what GitHub uses for Markdown files (e.g. README.md). What makes you say it prevents JS inside of SVG? <svg> isn’t even whitelisted.

kevinSuttle commented 8 years ago

Let me dig up the issue. It came up in one of their repos.

kevinSuttle commented 8 years ago

@mathiasbynens: via the 2nd link

This whitelist prevents authors from turning a Markdown-based readme into a whole website. HTML can be inserted, but with limited control over layout and style (and no ability to threaten security by running JavaScript).

Sorry, was implicit that SVG isn't whitelisted by design to prevent inline styling or layout by the sanitizer rules.

kevinSuttle commented 8 years ago

https://hacks.mozilla.org/2016/02/implementing-content-security-policy/ @bcoe could npm do something similar with CSP?

kevinSuttle commented 8 years ago

@bcoe Just occurred to me: Most README badges use SVG + URL params in GitHub.

e.g. [![Build Status](https://travis-ci.org/shipitjs/shipit.svg?branch=master)](https://travis-ci.org/shipitjs/shipit)

Seems pretty safe if they are uploaded and sanitized.

bcoe commented 8 years ago

@kevinSuttle I'm definitely interested in writing a little sanitization proxy that npm On-Site could route .svgs through. Seems like it would solve this problem, and it would be a great OSS module.

mathiasbynens commented 8 years ago

@kevinSuttle

Just occurred to me: Most README badges use SVG + URL params in GitHub.

Like I said before, <img src=svg> is always safe. You’d just have to host the SVGs on a different origin so they can’t execute scripts in the npmjs.org origin when accessed directly.

kevinSuttle commented 8 years ago

Ok then. Sounds like we have a general consensus?

kevinSuttle commented 8 years ago

https://www.npmjs.com/static/images/npm-logo.svg :trollface:

kevinSuttle commented 8 years ago

This might be helpful as well. https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

zeke commented 8 years ago

Back in December 2014, @seldo, @isaacs and I came up with some baseline expectations about how logos could work. See https://github.com/npm/newww/issues/291

The gist:

kevinSuttle commented 8 years ago

Nice thanks @zeke.

Side note and one final comment on SVG being vulnerable: anything can be vulnerable. https://fin1te.net/articles/xss-on-facebook-via-png-content-types/ http://m.instructables.com/id/How-to-Hide-Files-Inside-Pictures/

seldo commented 8 years ago

Those vulns aren't really the same though; the only reason the PNG was executable was because Facebook's CDN was incorrectly rendering it with an HTML content-type. PNGs rendered as PNGs can't execute JS.

kevinSuttle commented 8 years ago

One thing that occurred to me: why does npm have to host/serve icons at all?

seldo commented 8 years ago

Two reasons: performance and control. We get a lot of hits, so putting these images on your random shared web server is likely to bring it down, and control because we want to be able to verify that the thing being served as an image is indeed an image. If the end-user controls the content-type we could be vulnerable to the style of attack used on Facebook above.

kevinSuttle commented 8 years ago

So, what if they were required to be relative, included in the package, and only hosted by npm after processing is done? Again, I'm assuming that there is processing done on the files. I mean, you're a JS registry? How do you keep those secure?

seldo commented 8 years ago

All the plans we've had for supporting logos basically work as you're describing: use the content described (or even contained) in the package as an origin server, via a proxy that validates and filters, then served publicly via our CDN. It's not rocket surgery but not a priority right now.

kevinSuttle commented 8 years ago

Ah ok. Then @bcoe do you want to throw a label or milestone on this when you get a moment?

kevinSuttle commented 8 years ago

Bump https://github.com/iconic/SVGInjector#options