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 a section for checking if the web application properly hashes passwords before storing them in the backend. #728

Open
pinkLagoon opened this issue Mar 17, 2021 · 10 comments
Labels
help wanted new New content to write

Comments

@pinkLagoon
Copy link

What would you like added?
Sometimes web applications do not hash their passwords before storing them. It should be rare these days but I've come across this a few times and could not find an appropriate WSTG item that matches this finding. This seems like a very important thing to be checking for especially if you have access to the source code of the application.

@pinkLagoon pinkLagoon added help wanted new New content to write labels Mar 17, 2021
@ThunderSon
Copy link
Collaborator

Are you able to provide me a scenario where it might be a direct attack vector against web?
I see this as something done to protect the private data stored. The guide doesn't go into that. It's similar to the guide not going into CC details.
@kingthorin is there maybe something to do with configuring security measures on data storage that we can try and set? Maybe in the first chapters?

@1nf3rn0-H
Copy link

Yes I totally agree to @pinkLagoon. If the organization faces a data breach or even an insider threat scenario, the hashed passwords would be a second layer of security which could at least reduce the impact of the breach.
A very simple guide might be to check size of password after intercepting the traffic. I know this is very basic and would only work if the hashing occurs on the client side.
I would love to contribute to any such projects.

@ThunderSon
Copy link
Collaborator

We are clear on the need and purpose of hashing passwords.
Client-side hashing is out of the discussion of this issue. Hashing the password before it reaches the server makes that hash the password, requiring another hash run on the server to secure that. We're talking about proper database entry hashing.

My question comes around: I am doing a penetration test, how can I know how the password is being handled? (maybe timing attacks to see how much server resources are being used, etc.) -- different attacks that evolved into sensitive data disclosure.
Even if we knew that hashing is happening, is it secure and safe? We can't know that, we need to code review it.

This is one of the "The only way to review this is to code review it". Otherwise, a testing body doesn't have access to the code, there is no way to know about this (unless they got RCE, SQLi, data dumps, etc.)

@kingthorin this can maybe be added to the cryptography section, but then we'd be colliding with the Password Storage Cheat Sheet. This is an ASVS requirement and have its own cheat sheet, I am debating how the WSTG could actually add anything beneficial to this.

@elarlang @rbsec if you have any thoughts on this, they're more than welcome :)

@kingthorin
Copy link
Collaborator

The only way I can see that this might fit in is if the tester were to come across one of the few remaining systems that in 2021+ still emails users their passwords in plaintext when they use "Forgot Password" (which obviously implies they are stored as such).

@rbsec
Copy link
Collaborator

rbsec commented Mar 21, 2021

As @kingthorin, the only really reliable way to determine that they're not hashing passwords is if they ever send you an existing clear text password. This is probably something that should be highlighted in the password reset testing guidance. At the moment it says:

When passwords are reset they are either rendered within the application or emailed to the user. This may indicate that the passwords are stored in plain text or in a decryptable format.

But that's not really reliable - the flow could be:

  • Generate new password
  • Send to user
  • Hash and store in DB

Timing attacks are unlikely to be feasible from a blackbox perspective (especially given how fast MD5 is). You could maybe learn they're using specific hashes in a few cases (like the Bcrypt length issues) - but this could also just be truncation.

I absolutely agree that this is an issue, but 99% of the time it's not one you'll find in a pentest (unless you can find other issues such as SQLi or arbitary file download). Unless you've got code and are also doing some code review/whitebox testing - but that seems a bit out of scope of the guide?

@rbsec
Copy link
Collaborator

rbsec commented Mar 26, 2021

Thinking about this, there's a somewhat related issue about magic hashes that you can test for from a blackbox perspective. It's more about hash comparison than the specific algorithm used, although if they are vulnerable then it probably implies they're using a weaker algorithm (as any has that starts with a $ won't be vulnerable).

Is that something that would be worth adding?

@elarlang
Copy link

This "magic hashes" or "PHP type juggling" is something worth testing, but test-result can be only verified incorrect situation. It also requires, that there is mistake in PHP program code (if hashes are compared correctly, there is no problem with linked hashes). I could not take it in as hashing testing.

@github-actions
Copy link

Please comment if you are still working on this issue, as it has been inactive for 30 days. To give everyone a chance to contribute, we are releasing it to new contributors.

@github-actions
Copy link

Please comment if you are still working on this issue, as it has been inactive for 90 days. To give everyone a chance to contribute, we are releasing it to new contributors.

@github-actions
Copy link

Please comment if you are still working on this issue, as it has been inactive for 90 days. To give everyone a chance to contribute, we are releasing it to new contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted new New content to write
Projects
None yet
Development

No branches or pull requests

6 participants