psi-4ward / psitransfer

Simple open source self-hosted file sharing solution.
BSD 2-Clause "Simplified" License
1.48k stars 216 forks source link

Potential Security Issue #176

Open JamieSlome opened 3 years ago

JamieSlome commented 3 years ago

Hello,

We recently received a vulnerability disclosure against your repository. I couldn't find an e-mail to contact or a security process to follow, so created this issue instead.

If you would like me to e-mail over the details or put them on the GitHub Issue, I'm more than happy to facilitate this for you. Otherwise, you can access the advisory here.

It is private to you and the discloser of the report.

If you have any questions, let me know.

-- Jamie from huntr.dev

psi-4ward commented 3 years ago

You can limit the filesizes https://github.com/psi-4ward/psitransfer/blob/master/config.js#L60 but not the amount of buckets - thats correct. Probably one would create a PR for that?

In my opinion this is not really a security issue but more a vector for a DoS. Could also be solved on server-side (ie use a mount with limited space)

zer0h-bb commented 2 years ago

Hi,

I originally reported the issue. The issue mentioned in my report was about an IDOR where a potential attacker could download encrypted files and crack them offline. I think you received another report regarding a potential "DoS".

psi-4ward commented 2 years ago

So you mean pushing the password over the wire is more secure than decrypting on client side? Imho right if the password itself is weak. For me both solutions are okay. Maybe you would like to prepare a PR?

zer0h-bb commented 2 years ago

I will do it later thank you.

But I'm thinking of another solution, way simpler : simply adding authentication when accessing the encrypted json file as it's already done for the non json one.

http://localhost:3000/2507a65aac91  require a password but http://localhost:3000/2507a65aac91.json doesn't, that's the fundamental issue.

psi-4ward commented 2 years ago

hmm no http://localhost:3000/2507a65aac91 does not require a password - it renders the download-app, fetches the json and require the user to enter the password to decrypt the json

psi-4ward commented 2 years ago

pls review

zer0h-bb commented 2 years ago

hmm no http://localhost:3000/2507a65aac91 does not require a password - it renders the download-app, fetches the json and require the user to enter the password to decrypt the json

My bad sorry, it's been a year I didn't look into the code

Regarding the PR, looks good to me, now attackers can't Harvest now, decrypt later.

You can, if you want, validate the report on huntr.dev (https://huntr.dev/bounties/1-other-psi-4ward/psitransfer/) and submit your PR on huntr.dev as well to earn some bucks.

But I think @JamieSlome can do it manually too

psi-4ward commented 2 years ago

It's all somewhat like a workaround cause the support of FileWriter API is still not supported on FireFox and Safari so real E2E encryption is hard to implement (I've no idea how to do with large-file support). Mozilla has some similar tool for a while (iirc FireFox Send?) but I've tested it and one can only download files which dont exceed the local memory.

We thought about a fallback (with warning message for the downloader) to decrypt the file on the server for unsupported browsers but in the end it costs to much time (and money).

I've looked at hutnr.dev but didnt find any button to submit the PR

zer0h-bb commented 2 years ago

I think it's definitely possible to use the old mechanism (where the client fetch the .json file) and still use client side decryption, but the user woule receive the AES encrypted .json file if and only if he provides the correct password.

The issue will be solved without increasing server costs.

Regarding the PR, I think @JamieSlome will handle it way better than me, I don't know how huntr.dev looks like from a maintainer point of view.

psi-4ward commented 2 years ago

I think it's definitely possible to use the old mechanism (where the client fetch the .json file) and still use client side decryption, but the user woule receive the AES encrypted .json file if and only if he provides the correct password.

Makes no sense to me. If eve hijacks the request to data.json she has also the password to decrypt the ciphertext. So either dont put it on the wire or trust the wire???

zer0h-bb commented 2 years ago

I may be totally wrong but the original issue I saw, was that if some one use your project on a publicy available webserver. Then any attacker could download AES encrypted uploaded files and then crack them offline, because no password was required to download these encrypted files. Attackers here have the ability to download every encrypted files in the server, provided they correctly bruteforce the little id identifying the files.

If you use authentication in order to download the encrypted document then decrypt offline, the attackers won't be able to download every uploaded files.

The eavesdrop attack you are mentioning (eve hijacking data.json) implies that the communication channel between a user and the server is flawed in the 1st place. Most websites use HTTPS, I don't think this is the application responsibility to take into account the fact that users don't use appropriate communication channels.

Anyway, requiring a password before downloading data.json drastically reduce the extent of the damages.

psi-4ward commented 2 years ago

Then any attacker could download AES encrypted uploaded files and then crack them offline, because no password was required to download these encrypted files.

There is NO encryption of the files itself. There are two (or with password 3) secrets to obtain:

So the attacker has to get knowledge of the bucket ID to download the files-list, crack it and download the plain files before the download expires and the files have been deleted. Both, bucket-ID and fiel-ID are derived from a UUID which is hopefully really hard to guess.

...implies that the communication channel between a user and the server is flawed

Yes ... when there is an eavesdropper or man-in-the-middle he probably could also obtain the bytes when the user downloads a file...