Uppy: Security issue

[lirantal]

Hi,

I'm a member of the Node.js Security WG and we received a report regarding a security issue with this module.

We tried inviting the author by e-mail but received no response so I'm opening this issue and inviting anyone with commit and npm publish rights to collaborate with us on a fix.

[goto-bus-stop]

Hi, thanks! We're having a look right now—it seems to us like a backend issue, not with uppy itself, since the actual vulnerability is when someone allows user-uploaded content with scripts to be accessed on their domain unfiltered. I don't know if we can do anything inside Uppy except warn in the documentation. Our demo tusd server does suffer from this problem too tho, so we're looking into addressing it there.

[lirantal]

No problems, happy to see you guys active on this.

Can you please give me an e-mail address to add you to the issue conversation on the Hacker1 platform?

[arturi]

I actually read the thread there, I got the invitation, thank you! I just hesitated to register :)

[lirantal]

Great, so let's continue the discussion there :)

[kiloreux]

@lirantal Can you please try now again? I have deployed a patch that should fix it.

[ghost]

Could we, ordinary mortals, also know what the problem is?

[kvz]

We don't have a commit to link to yet, but we're experimenting with this Nginx config on the tusd instance:

# Security: Don't allow people to upload html and then have browsers parse it
if ($uri ~ \.(?!(jpe?g|gif|png|webp|webm|mp4|mpg|avi|3gp|wav|mp3))$) {
    add_header Content-Type application/octet-stream;
    add_header Content-Disposition "attachment; filename=$basename";
    add_header X-Download-Options noopen;
    add_header X-Content-Type-Options nosniff;
}

To avoid that upon serving out uploaded files, html, svg, js, etc are parsed & interpreted by the browser. Because, at least in our tus and Uppy demos, anyone could upload anything.

[kvz]

In most apps, it might make sense to reject html and js via https://uppy.io/docs/uppy/#restrictions (but that's only a client side restriction, it would be wise to also reject this content on the server)

[lirantal]

@kiloreux could you jump in on the Hacker1 platform for the conversation? I'm stressing you'd do that so that you can collaborate with the bughunter who found it, they can confirm it, or give you further feedback if something is still off.

Keep in mind that I want you to do it there to follow a responsible disclosure policy which means you have enough time to fix the security issue without it getting into the wild and put users of your project at risk. (cc @johnunclesam)

[kvz]

There's some discussion within our team if this actually fixed the issue since Nginx might not be serving out these files, but instead, tusd is doing so directly: https://github.com/tus/tusd/commit/755e892e3039f333f2738d44749e6934af8c189e#r27614413

I should have known this but I didn't have it available in my brain so I apologize for the misdirection that I have caused. Luckily/hopefully, the nginx fix will prove fruitful for those cases where Nginx _is_ tasked with serving out uploaded content so it's not all for nothing. I've asked @Acconut to shine a light on this issue since, as author of tusd, he'll know how to best address this.

[goto-bus-stop]

@lirantal

Can you please give me an e-mail address to add you to the issue conversation on the Hacker1 platform?

My email is [email protected]. thanks!

[kvz]

Can you please also invite [email protected] and [email protected] in one go?

--
Kevin van Zonneveld (kvz.io)
Co-founder at Transloadit (transloadit.com)
Core-author of tus (tus.io)

On 19 February 2018 at 10:50:16, Renée Kooi ([email protected]) wrote:

@lirantal

Can you please give me an e-mail address to add you to the issue conversation on the Hacker1
platform?

My email is [email protected]. thanks!

--
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/transloadit/uppy/issues/632#issuecomment-366638122

[lirantal]

Yes, I'll invite all of you so we can pick it up there!

[lirantal]

FYI no one joined the invite and the bug hunter confirms the issue is still valid. We'll proceed with a public disclosure in a couple of weeks but I really want you guys to be on the discussion.

[kvz]

Sorry about the delay here people, I did get an invite to hackerone and accepted it, and was able to see the thread. I believe it was waiting for the researcher to confirm it was fixed at that time. When I try to visit the thread now with my [email protected] account, I get this:

I should have been more pro-active here and ask if this was fixed already, I'm sorry about that.

After fixing it in an ineffective way, earlier (11 days ago)
https://github.com/tus/tusd/commit/755e892e3039f333f2738d44749e6934af8c189e

Our proper fix (or so we think) has been live since eight days ago:
https://github.com/tus/tusd/commit/6230c23566adaff4b74ce4ec9b572dba39adc569

Earlier, we mistakenly thought Nginx served out the files, but it was tusd.. but now, both servers will refuse to let browsers interpret SVG, and other risky formats.

Sadly since I lost access to that Hackerone thread I also lost access to the SVG file that the vulnerability researcher tested with. However, upon testing with my own 'evil SVG', you'll see that the browser now downloads the file, instead of interpreting it. I'm thinking that, if this works for one SVG, it should for another.

Here's how I'm reproducing it:

Is the vulnerability research perhaps using a different location than e.g. https://uppy.io/examples/dashboard/ to test with? Or did he test it between our bad/good fix maybe?

Again, sorry this thread has been so slow in moving forward. We had a few different components to look at, holidays, sickness, but as I hear myself talking we really don't have a valid excuse to not be 💯 on top of this

[Acconut]

@lirantal All of our invitation links expired without any notice. Would you be so kind and resend them? We used them to read the thread on Hackerone but were not award that they are only temporary.

[lirantal]

Ok so I re-sent the invites, glad to see you guys are still committed on a conversation and a fix for the problem.

[arturi]

@lirantal I just received a revocation email, no new invitation: “Your invitation to participate on * was revoked”. Please re-send for me too: [email protected]. Thanks!

[lirantal]

yeah because I had to revoke it and then re-add the e-mails.
Check the inbox again to see if you received the new invite.

[arturi]

I did, thank you!

[arturi]

Going to close this, since the issue was not on Uppy’s side, but on tusd server side, and it is currently patched. Thank you @Alyssa-o-Herrera for finding it and @lirantal for managing and reporting too!