First Flight #1: PasswordStore

Summary

I found 2 high risk vulnerabilities that allows anyone to set a password and view it on-chain eventhough a few checks are in place to supposedly prevent this.

Vulnerability Details

First Vulnerability - https://github.com/Cyfrin/2023-10-PasswordStore/blob/main/src/PasswordStore.sol#L26-L29
In this code snippet, there is no mechanism to only allow the owner to set the password, anyone can set a new password. Proof is shown here: https://github-production-user-asset-6210df.s3.amazonaws.com/66850474/277986522-ba3422d4-4ade-4a2f-909c-26643925a4de.png

Second Vulnerability - https://github.com/Cyfrin/2023-10-PasswordStore/blob/main/src/PasswordStore.sol#L14
In this code snippet, the variable is marked as private and is not intended for others to see, but if anyone were to track the state changes at slot 1 which is where the s_password variable is stored, they would be able to track the password value as shown in the picture here: https://github-production-user-asset-6210df.s3.amazonaws.com/66850474/277987680-4f592ad6-a980-4e24-aaaf-48837707cb6c.png

Impact

It is not best practice to store private data on chain as its generally viewable publicly, and in the case of anyone being able to set the value, it will alter the logic of the contract so anyone will be able to control it.

Tools Used

Remix IDE, Etherscan, Goerli

Recommendations

First Vulnerability - include modifiers like onlyOwner() or require statements to ensure proper access control, go for the methodology that results in lesser gas fees.

Second Vulnerability - don't store sensitive data on chain, another way to go about it is store the encrypted version on chain and maybe sign it with a private key to be unlocked later on.