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

Add instructions for protection against vulnerabilities #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maarten-litmaath
Copy link

Hi,
I think we need implementations to take steps to protect users and others
against accidental or deliberate abuses via world-writable directories,
to the extent possible. If someone else e.g. creates /tmp/bt_u$MY_UID,
then the code must not consider that file, but rather fail with a warning...

Hi,
I think we need implementations to take steps to protect users and others
against accidental or deliberate abuses via world-writable directories,
to the extent possible.  If someone else e.g. creates /tmp/bt_u$MY_UID,
then the code must not consider that file, but rather fail with a warning...
@DrDaveD
Copy link
Contributor

DrDaveD commented Sep 16, 2020

It's easy to understand why one needs to make sure that writing doesn't follow symlinks created by someone else, but what is the vulnerability if someone creates a token for someone else? What kind of harm could be done, for example?

@maarten-litmaath
Copy link
Author

Hi Dave,
at the very least we should try to avoid having to debug perplexities like:
"how can John Doe's token end up being used for my request?!"
Better fail much earlier in the chain. I do not think there is a valid
use case for someone preparing a token for another (in such a way).
I suspect that in principle worse abuses would be possible,
but I do not have something realistic beyond a DoS yet.

@DrDaveD
Copy link
Contributor

DrDaveD commented Sep 16, 2020

In that case maybe the MUSTs should be changed to SHOULDs.

@msalle
Copy link

msalle commented Sep 17, 2020

Note that if the file isn't owned by the user it typically also means it needs to be group or even world-readable. I think something like that would only be acceptable on servers where users don't have access. For any shared system (UIs, WNs), I think a MUST is the correct one. Note that we have the same for proxy files: they need to be readable only for the owner.
As an example of an attack scenario: if a user would upload private data with an attackers credentials, the attacker then has access to that private data.
In short I think a proper ownership check + file permissions (incl. the standard paths checks to prevent symlink attacks) is necessary on any shared system.

@DrDaveD
Copy link
Contributor

DrDaveD commented Sep 17, 2020

Well then if we're getting into requiring security checks in every token reader, it ought to be complete and require that the file is only accessible by the current user.

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

Successfully merging this pull request may close these issues.

3 participants