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

setPassword function does not check if msg.sender is s_owner

zen4269

Summary

setPassword function does not check if msg.sender is s_owner, meaning that anyone can change the password by calling this function

Vulnerability Details

Lack of checks mean attacker can change password previously saved by the owner as the test below illustrates. This test should have failed in a properly implemented contract, but with the current state of the contract, it passed.

contract PasswordChangedNotOwnerTest is Test {
PasswordStore public passwordStore;
DeployPasswordStore public deployer;
address public owner;
address public attacker = vm.addr(1);
​
function setUp() public {
deployer = new DeployPasswordStore();
passwordStore = deployer.run();
owner = msg.sender;
}
​
function test_non_owner_can_set_password() public {
vm.startPrank(owner);
string memory expectedPassword = "myNewPassword";
passwordStore.setPassword(expectedPassword);
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, expectedPassword);
vm.stopPrank();
​
vm.prank(attacker);
console.log('attacker setting u password');
string memory newPassword = 'attackerPassword';
passwordStore.setPassword(newPassword);
//need to change back to owner to see password
vm.prank(owner);
console.log('owner check new password');
string memory actualChangedPassword = passwordStore.getPassword();
assertEq(actualChangedPassword, newPassword);
}
}

Impact

The contract is not a reliable tool to save private password as it could be changed by attacker, and getPassword will then return a different password than originally saved by the owner

Tools Used

Foundry

Recommendations

Add the check that msg.sender is s_owner, the setPassword function should be modified as follow:

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.