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

fix: don't expose user existence if visitor can't view users #12447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oplik0
Copy link
Contributor

@oplik0 oplik0 commented Mar 27, 2024

resolves #12432

This is a slightly hacky way to fix this, but I'm not entirely sure if there aren't any plugins depending on exposeUid/exposeGroupName short-circuiting to a 404 on lack of the relevant value (which obvious solutions would need to change), while this ensures it still goes directly to 404 if the visitor has the relevant privilege while going to notAllowed otherwise.

This way also required changes to just the middleware and not any of its consumers.

Also, this fixes the same issue for groups because the code was shared already.

@barisusakli
Copy link
Member

Do we really want to do this? Can't guests just go to /register page and check if a username is already taken by trying out different usernames?

@oplik0
Copy link
Contributor Author

oplik0 commented Mar 27, 2024

Unlike just loading the user page, registration is much more likely to be protected by e.g. CAPTCHA than a normal page load, so checking that would be much more taxing on an attacker than a simple URL enumeration (and even without special protection, submitting a form with CSRF requires a bit more work from the attacker).
You can also limit registration, either by making the forum invite only, or even disabling it for the duration of such an attack. And it's more reasonable to apply very strict rate limits there.

I honestly can't say whether it's really important though, but it is a sort of non-obvious information leakage and I can see some scenarios where it could be useful (e.g. invite-only forum that's trying to only expose some announcement/information [like how to get an invite] topics to guests but hides most content and doesn't want to expose a user list).

However, looking again at the issue it seems I solved Y instead of X - the actual problem the user is experiencing is high traffic from a malicious user, arguably a better solution there could be a rate limit...

@diamante0018
Copy link

Unlike just loading the user page, registration is much more likely to be protected by e.g. CAPTCHA than a normal page load, so checking that would be much more taxing on an attacker than a simple URL enumeration (and even without special protection, submitting a form with CSRF requires a bit more work from the attacker). You can also limit registration, either by making the forum invite only, or even disabling it for the duration of such an attack. And it's more reasonable to apply very strict rate limits there.

I honestly can't say whether it's really important though, but it is a sort of non-obvious information leakage and I can see some scenarios where it could be useful (e.g. invite-only forum that's trying to only expose some announcement/information [like how to get an invite] topics to guests but hides most content and doesn't want to expose a user list).

However, looking again at the issue it seems I solved Y instead of X - the actual problem the user is experiencing is high traffic from a malicious user, arguably a better solution there could be a rate limit...

I agree with your explanation, the forum I use is protected by recaptcha so it would be extremely difficult for someone to test out if a username is taken that way. Unfortunately, that's all I can say for this matter as I'm not familiar with the technicalities behind this PR

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.

Guest users still can determine whether a userslug exists or not
3 participants