-
Notifications
You must be signed in to change notification settings - Fork 647
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
KeyStore DoS vulnerability to crash the node [BugBounty][Security Issue] #195
Comments
Some additional note why I think 1024 limit is too high and not worth it at the cost of security. (26{lower alpha}+26{Upper}+10{numbers}+30{special charaters})^8 If we go for 25 as upper limit as submitted in PR It's still quite a huge/impossible possibility to brute force. |
If one accepts the current maximum user/pass length, this is 2kb in size which is not much data at all. So if the RPC server is being DOS'd from a ~2kb request this means there is some other underlying issue. By reducing the maximum field length, this does NOT fix the underlying issue, rather it avoids it. Therefore you would investigate further please to find the real cause as other types of RPC calls could be effected similarly. |
Also for your Reproduction step 3, how many POST requests are you making per second? How many requests are made in the 5 seconds the RPC server becomes unresponsive? |
Hi @swdee Regarding step 3: I realized the fix I submitted to reduce the value of |
@Shashank-In Thanks for explaining what your seeing further, however I disagree that the password length has any effect on the hashing of the password. The hashing method used is Argon2 at the code point here https://github.com/ava-labs/gecko/blob/83502ee59e19bd93a2205753d3ff317bcde4c4a8/api/keystore/user.go#L26 I put together a quick benchmark comparing hashing of a password of length 1024 characters versus 25 and you can see there is no real difference. Code here https://github.com/swdee/argonbench with results
A single run irrespective of the password length takes around 60ms on my workstation. As Argon2 was designed to be resistant to GPU cracking, it requires the allocation of a large amount of memory, which has been configured to be 64MB. If you are firing 50 threads, this would require [64*50] 3.2GB RAM to support, so the slow down your seeing I suspect is your RAM is exhausted and your system has gone into Swap which is putting a huge i/o load on the system. So the proper fix would be to rate limit the user creation API to avoid the DOS and not change the password length which weakens the security without any effect. |
@swdee Sorry I am still confused why did a password with 1024 characters takes 42 seconds and 635 characters take 12 seconds? Note: I am not firing any threads. Just a single request. Here is my observation. (I have 8GB ram and the observation was done multiple times for each request) username: test102 Request
username: test103 Request
username: test104 Request
username: test100 Request
I am still unsure if rate-limiting would be very helpful. I guess first the problem is why the response time is increasing exponentially when password length is increased. |
Also @swdee I had the same observation with a slight difference on avawallet website. Note: I went really cautious and did not cause any disruption. Password length: 512
In localhost it was 5.5 seconds for me and on the wallet website, it was 7seconds and all the response time was quite similar. I would be glad to know your views. |
@Shashank-In I haven't dug any deeper yet to see why it slows, but have just updated this here to identify the cause. |
Going back to the source library it has a note about latency here https://github.com/dropbox/zxcvbn#runtime-latency
So if we truncate the password to the first 100 characters that gets passed to zxcvbn() we can avoid the slow down and still test password strength using this library. So by changing; To the following would provide a suitable fix.
Limiting the check to 100 chars consumes around 20ms of execution time on my workstation. Try it out and let me know if it solves your issue? |
Waoo that's quite helpful. I will update the code in my localhost and comeback with a new PR and my observations. Thank you @swdee for your insights. |
Hi @swdee
I have added checks for pass length and submitted a PR |
Describe the bug
The keystore has an API to create a user with input values
username
andpassword
It was noticed that the username and password have an upper limit of 1024 characters.
However, 1024 characters are too much for a password and is causing a denial of service bug
The request to create an account is
It was noticed that when a 635 character password is used it takes almost 12 seconds however when a 1024 character password is used it took 42 seconds which is too much.
A malicious user can make multiple such requests and make the service unavailable.
To Reproduce
./build/ava
This can be confirmed by making any keystore request like
We will notice we won't get any response back.
Note: The node has to be restarted to get make everything work again.
Expected behavior
The node should be able to handle such concurrent request as well as the max length of password should be reduced
Screenshots
Operating System
MacOS, ubuntu
Additional context
It was tested on my localhost. After 5 seconds the server was stuck and won't be responsive
Suggested Fix
Since we are already doing following strong password policy. A password length of even 25 is enough.
By submitting this issue I agree to the Terms and Conditions of the Developer Accelerator Program.
The text was updated successfully, but these errors were encountered: