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

Lack of isOwner check on PasswordStore::setPassword method enables anyone to modify the password set by owner of the contract.

0xspryon

Summary

Lack of isOwner check on PasswordStore::setPassword method enables anyone to override the password that was set by the owner.

Vulnerability Details

Lack of isOwner check on PasswordStore::setPassword method enables anyone to override the password that was set by the owner thus, breaking the assumption that the contract owner can safely store and retrieve his password because the owner is not guaranteed to always retrieve the exact password he stored in the contract.

POC

Include the below code excerpt in test/PasswordStore.t.sol and run in the terminal
forge test --match-test test_non_owner_can_set_password

function test_non_owner_can_set_password() public {
address randomUser = address(1);
// owner sets password
vm.startPrank(owner);
string memory expectedPassword = "myNewPassword";
string memory notExpectedPassword = "notExpectedPassword";
passwordStore.setPassword(expectedPassword);
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, expectedPassword);
vm.stopPrank();
//randomUser overrides password
vm.prank(randomUser);
passwordStore.setPassword(notExpectedPassword);
vm.prank(owner);
string memory newActualPassword = passwordStore.getPassword();
//passwordStores.s_password has been overriden
//to the new value set by randomUser
assertEq(newActualPassword, notExpectedPassword);
}

Impact

The assumption is, the contract owner can safely store and retrieve what he stored. That assumption is broken because anybody can override what the contract owner stored.

Tools Used

manual review

Recommendations

Only set the password if the msg.sender == 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
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.