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

`PasswordStore::setPassword` function allows anyone to set a password

maroutis

Summary

The PasswordStore::setPassword function allows anyone to execute the function and set a new password.

Vulnerability Details

/*
@> * @notice This function allows only the owner to set a new password.
* @param newPassword The new password to set.
*/
@> function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}

As mentioned in the comments, the password value is not supposed to be accessible by entities other than the owner. However, the function does not restrict access. As of now, anyone who executes this function can change the password.

Impact

Anyone can change the password ! If the password is used to lock/unlock something important like a vault, the protocol will be completely compromised.
For the contract PasswordStore, the core functionality which limits access to the PasswordStore::setPassword function for setting up a new password is compromised.

Proof of Concept

You can execute the following test using the command : forge test --mt test_non_owner_can_set_password -vvvv

function test_non_owner_can_set_password() public {
address hacker = makeAddr("Hacker");
​
vm.startPrank(hacker);
​
// set up a new password
string memory NewPassword =
"A very very very very strong secret password";
passwordStore.setPassword(string(abi.encodePacked(NewPassword)));
vm.stopPrank();
​
vm.startPrank(owner);
string memory password = passwordStore.getPassword();
vm.stopPrank();
​
// Check that set up function has been compromised
assertEq(NewPassword, password);
}

Tools Used

  • foundry

Recommendations

Restrict access to the function PasswordStore::setPassword. There are many ways to do this :

  • Check that msg.sender is the owner :

- function setPassword(string memory newPassword) external {
- s_password = newPassword;
- emit SetNetPassword();
- }
+ function setPassword(string memory newPassword) external {
+ if (msg.sender != s_owner) {
+ revert PasswordStore__NotOwner();
+ }
+ s_password = newPassword;
+ emit SetNetPassword();
+ }

  • Add a modifier that restrict function use to the owner. The onlyOwner modifier from Openzeppelin can be used for this purpose :

- function setPassword(string memory newPassword) external {
- s_password = newPassword;
- emit SetNetPassword();
- }
+ function setPassword(string memory newPassword) external onlyOwner {
+ s_password = newPassword;
+ emit SetNetPassword();
+ }
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.