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

Malicious user can set the password since there is no access control for `setPassword`

gabr1sr

Summary

The setPassword function within the PasswordStore contract displays a critical vulnerability due to the lack of access control. This vulnerability allows a malicious actor to set the stored password.

Vulnerability Details

In the setPassword external function, a string value named newPassword is received as argument, used to set the s_password private string variable and an event is emitted. But, since there is no code checking if msg.sender is equal to s_owner, anyone can execute this function and set the password.

function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}

Proof of Concept

Paste the following code inside the PasswordStoreTest contract located at test/PasswordStore.t.sol file:

function test_non_owner_set_password() public {
vm.startPrank(address(1));
string memory newPassword = "myNewPassword";
passwordStore.setPassword(newPassword);
vm.stopPrank();
​
vm.startPrank(owner);
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, newPassword);
}

And run forge test --match-test test_non_owner_set_password. We can see that the test run successfully as it doesn't revert.

The code block above:

  • Impersonates the address(1) which is not the contract owner

  • Creates a new variable newPassword storing the new password "myNewPassword"

  • Executes the setPassword function using the newPassword variable as argument

  • Impersonates the owner address of the PasswordStore contract

  • Gets the password from PasswordStore contract using getPassword function and store it inside the actualPassword variable

  • Checks if the newPassword value is the same as the actualPassword value

Impact

A malicious user can change the stored password and make the owner use a incorrect password, losing access to the account the password is used for.

Tools Used

Visual Studio Code and Foundry.

Recommendations

Make the function setPassword revert with PasswordStore__NotOwner error if the caller address (msg.sender) is not equal to the owner address (s_owner):

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

Lead Judging Commences

inallhonesty Lead Judge
about 2 years ago
inallhonesty Lead Judge about 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.