Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Invalid

The `newPassword` parameter in `setPassword()` function can be the old password

Summary

The owner can set the previous password as a newPassword in the setPassword() function.

Vulnerability Details

The setPassword() function in contract PasswordStore.sol allows the owner to set the previous password as new password. The function doesn't perform any checks of the input argument newPassword, directly assign the new value to the variable s_password:

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

Impact

The setPassword() function in contract PasswordStore.sol does not checked if the input argument newPassword is not the previous password. If a owner make a mistake and write the previous password as input argument, the function will not complain about that. This way the function does nothing, only the owner pays for the transaction used.
The following test function demonstrates this issue. The function can be added to the file PasswordStore.t.sol and executed in Foundry using the following command: forge test --match-test test_owner_can_set_the_previous_password

function test_owner_can_set_the_previous_password() public {
vm.startPrank(owner);
string memory expectedPassword = "myPassword";
passwordStore.setPassword(expectedPassword);
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, expectedPassword);
}

The result is that the test is passed:

[PASS] test_owner_can_set_the_previous_password() (gas: 19318)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.28ms

Tools Used

VS Code, Foundry

Recommendations

Add a check that the input argument newPassword is not the same as the previous password.

function setPassword(string memory newPassword) external {
+ require(keccak256(abi.encodePacked(newPassword)) != keccak256(abi.encodePacked(getPassword())), "The new password should not be the same as the previous one");
s_password = newPassword;
emit SetNetPassword();
}

The function getPassword() should be declared as public to use it in the setPassword() function.

Updates

Lead Judging Commences

inallhonesty Lead Judge
almost 2 years ago
inallhonesty Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: User input validation

Support

FAQs

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