jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.47k stars 4.02k forks source link

Rate limit for password reset #20072

Open manthanhacker1 opened 1 year ago

manthanhacker1 commented 1 year ago

Missing rate limit for current password field (Password Change) Account Takeover on https://start.jhipster.tech/

Vulnerable site : https://start.jhipster.tech/

Steps to reproduce the bug:

1)Go to Account> Change Password. Enter any (wrong password) In the old password field. 2)Now enter the new password and Turn the Intercept ON. 3)Capture the request & Send the request to Intruder and add a Payload Marker on the current password value. 4)Add the payload for the password field having a list of more than 200 passwords or more for the test and start the attack.

Impact : There is no rate limit enabled for the "Old Password" field on changing password on your website. A malicious minded user can continually try to brute force an account password. If a user forgets to logout an account in some public computer then the attacker is able to know the correct password, and also able to change the password to a new one by inputting a large number of payloads.

Mitigation : Add rate limit on Old password field.

atomfrede commented 1 year ago

Same as for https://github.com/jhipster/generator-jhipster/pull/20074 applies here.

deepu105 commented 1 year ago

@atomfrede @mshima @manthan1345267 contacted me and julien via mail to report vulnerability and since we didn't feel these are CVE issues I asked to do PRs so I'm gonna give him benefit of the doubt. @manthan1345267 could you please do PRs with actual implementation as the one you opened looked like spam or incomplete

atomfrede commented 1 year ago

Good to know. Sorry for marking it as spam. As I said the point is valid but the pr looked like spam.

mshima commented 1 year ago

Great, I've closed the PRs because:

They are PRs to merge our own branches see:

Captura de Tela 2022-10-22 às 07 24 08

We should be careful because such huge PRs can mask credentials stealing at GitHub Actions.

But the implementation will be nice.

atomfrede commented 1 year ago

Just to get it clear, by rate limiting on old password field you mean rate limit the "change password" endpoint. So e.g. after 5 failed attempts forbid to do it again for e.g. 1h (or longer) for e.g. the malicious ip/server whatever?

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for too long without any activity. Due to the moving nature of jhipster generated application, bugs can become invalid. If this issue still applies please comment otherwise it will be closed in 7 days

atomfrede commented 8 months ago

Please keep it open as it might be a good thing to implement

manthanhacker1 commented 8 months ago

Just to get it clear, by rate limiting on old password field you mean rate limit the "change password" endpoint. So e.g. after 5 failed attempts forbid to do it again for e.g. 1h (or longer) for e.g. the malicious ip/server whatever?

Absolutely, you're on point! Rate limiting on the "change password" endpoint is an effective way to prevent brute force attacks. After a certain number of failed attempts (like 5), the system can restrict further attempts from the same source (IP address or server) for a designated period, such as an hour or longer.

Regarding your request for a pull request on GitHub, I appreciate your understanding. As a security researcher focused on identifying and reporting vulnerabilities, I'm not familiar with creating PR's so if anyone can help or collaborate from team to create PR it'll be great.

regards, Manthan

atomfrede commented 8 months ago

@jhipster/developers I have started to look into Bucket4J for JHipster-Online and it looks quite good as we can use the sql db and don't need hazelcast. I would try to finish it for jhipster online. We might integrate that into the generator itself (problem, not all database are supported) or we just document it how to enable it.

pmverma commented 8 months ago

A few years back, I integrated Bucket4J through https://github.com/MarcGiffing/bucket4j-spring-boot-starter in a JHipster-based spring MVC backend. It was super easy to integrate and the repo has great documentation as well.

atomfrede commented 8 months ago

Interesting @pmverma. For jhipster online this won't work as we don't have a distributed cache, thats why I looked at the database support.

pmverma commented 8 months ago

From what I can see in the Jhipster Online git repository, it already uses caching with ECache, then it would be much easier IMO. The Getting Started section shows examples with the same ECache as well.

For jhipster online this won't work as we don't have a distributed cache, thats why I looked at the database support.

It doesn't have to be a distributed cache. I am not sure why are you looking for DB support. Am I overlooking something?

atomfrede commented 8 months ago

If it is not distributed across nodes the throttling will be per deployed instance. If we have just one running instance thats fine of course.

pmverma commented 8 months ago

@atomfrede Sorry for the late reply. Isn't JHipster Online running on a single instance? I don't have console access, but I am talking from the point of view that EhCache is used and hence my understanding is it is a single instance app.

atomfrede commented 8 months ago

Maybe you are right. I thought we have at least two instances. So maybe I will give the starter a try. With regards to the generator ( in case we want it to be generated) the database approach should at least be evaluated.

github-actions[bot] commented 2 months ago

This issue is stale because it has been open for too long without any activity. Due to the moving nature of jhipster generated application, bugs can become invalid. If this issue still applies please comment otherwise it will be closed in 7 days

atomfrede commented 2 months ago

Keep it open