First Flight #1: PasswordStore
First Flight #1
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`setPassword()` missing access control check, anyone can set the password

stakog

Summary

The setPassword() function does not limit who can call it and set a new password, but the purpose was that only the owner of the contract would be able to set a new password.

Vulnerability Details

The function setPassword() is used to set a new password for the owner of the contract and store the new value in the s_password state variable. Since it's the owner's password, the function should limit itself to be called only by the owner. This limitation is also documented in the comments where it is stated @notice This function allows only the owner to set a new password. But this limitation does not exist, no check on who called the function and the function is set to public, so anyone can call this function and set a new password for the owner.

Impact

Anyone can change the owner's password which is directly in contrast to its purpose as stated in the comments.
We don't really know what the password is used for, but passwords are usually used to protect sensitive information or funds. Therefore, a compromised password very often means a lot of damage. Also, as can be seen in the test code provided below, it's straightforward to change the password. This is why the severity for this bug is high.

Here is a test function with comments, to show that an attacker (non-owner) can set s_password to whatever he or she wants:

function test_non_owner_can_set_password() public {
​
// set new password as attacker
vm.startPrank(attacker);
string memory expectedPassword = "newAttackerPassword";
passwordStore.setPassword(expectedPassword);
vm.stopPrank();
​
// read currently stored password as owner
vm.prank(owner);
string memory actualPassword = passwordStore.getPassword();
​
// verify that the password the owner reads is the same one the attacker set.
assertEq(actualPassword, expectedPassword);
}

Tools Used

  • Foundry

Recommendations

To mitigate the issue, add an access control check on the caller of the function -> require(msg.sender == owner).

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-lacking-access-control

Anyone can call `setPassword` and set a new password contrary to the intended purpose.

Support

FAQs

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