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

Rate limit on password resets #4993

Closed
1 task done
KrisLowet opened this issue May 11, 2024 · 11 comments
Closed
1 task done

Rate limit on password resets #4993

KrisLowet opened this issue May 11, 2024 · 11 comments

Comments

@KrisLowet
Copy link

Describe the feature you'd like

Currently, there is no rate limit for resetting passwords. Unlimited addresses can be entered.
An idea is to limit the resetting password feature for IP's that requests new passwords for non-existing accounts.

Describe the benefits this would bring to existing BookStack users

More security due to blocking malafide requests.

Can the goal of this request already be achieved via other means?

A captcha method.
Logging resets for unknown emails addresses (like logging failed access) to block the IP via failed2ban.

Have you searched for an existing open/closed issue?

  • I have searched for existing issues and none cover my fundamental request

How long have you been using BookStack?

1 to 5 years

Additional context

No response

@samadha56
Copy link
Contributor

I am interested in implementing this feature for the project. This feature will provide rate limiting for password reset requests based on IPs that submit excessive requests for non-existent accounts, thereby enhancing the overall security of the project.

To implement this feature, I will utilize technologies such as CAPTCHA and logging for suspicious IPs to effectively identify and prevent abnormal requests, thus mitigating potential malicious attacks.

I can provide further details regarding the implementation specifics and associated costs after conducting a more thorough review and understanding of the project requirements. I am ready to enhance the security of this project by incorporating this valuable feature.

Thank you for considering me for this opportunity, and I look forward to your response.

@adriantirado
Copy link

hi this vulnerability would be valid to be recognised as a cve

@ssddanbrown ssddanbrown added this to the v24.05.1 milestone May 11, 2024
@ssddanbrown ssddanbrown self-assigned this May 11, 2024
@ssddanbrown
Copy link
Member

Thanks for raising @KrisLowet, I agree that a rate limit would be ideal here, and also a random delay if not already there.
I've assigned this to be something for our next patch release.

@samadha56 Thanks for the offer but please don't provide a PR. Your message indicates you may go down a more complex path than needed and this will be something I'd want to merge soon so is something I'd take on myself.

@adriantirado
Copy link

hi this vulnerability would be valid to be recognised as a cve
thanks you

@ssddanbrown
Copy link
Member

@adriantirado Okay, you repeated the same message as above. Or are you asking if we'll create a CVE?

I've always had trouble with the CVE process, and lack of control of CVEs. In the past, they been opened by bounty platforms or the reporter via their own CNA process. I did open some CVEs through GitHub before but I'm not fully keen on their process and don't really want deeper reliance on GitHub. Maybe something we need to spend time on to formalize, but I remember having trouble understanding CVE management when looking before.

@adriantirado
Copy link

hi so you won't create a CVE for me? and how else could it be formalised, you can try it now, maybe it will work out well?

thanks

@adriantirado
Copy link

hi is it possible to publish it myself from the cveform page, you have to give me the data that it asks me for the version for example, if you accept it I will give you the information that I need to complete the cveform

thanks you

@adriantirado
Copy link

hi

@ssddanbrown
Copy link
Member

@adriantirado I've opened #5004 to better think through and formalise our security announcement & CVE process. I've opened this when thinking about CVEs from the above, and since I'm not sure in cases like this if such an issue/change is something within the scope of what we'd announce since to me this is improving/hardening security rather than fixing a vulnerability. Even adding IP-based rate-limiting, the same exploit could still be used but just at a higher cost/effort.

If you have experience in this area (especially in open source), feel free to add your comments in #5004 to help build that process.

ssddanbrown added a commit that referenced this issue May 21, 2024
Some already throttled in some means, but this adds a simple ip-based
non-request-specific layer to many endpoints.
Related to #4993
@ssddanbrown
Copy link
Member

This has now been added within 69af9e0, and will be part of the patch release to be soon release. This includes a 10 per minute per-IP request limit, in addition to a random pause period during request handling.

Thanks again @KrisLowet for raising this request.

@ssddanbrown
Copy link
Member

In the end I made v24.05.1 a security release, and was therefore announced as a security release.
If it was the lack of these referenced rate limits alone, I would not have been too concerned (since we do have rate limits on known emails) but this led me to additional and more substantial concerns in how some other endpoints could be used without limit, and therefore I wrapped up these together into a security release.

I am also testing out requesting CVEs directly with mitre for this, and have requested a CVE ID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants