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

[H-02] Unauthorized Password Alteration due to Missing Access Control in `setPassword` Function

maanvad3r

Summary

The PasswordStore contract is designed to securely store a password while allowing only the contract owner to modify it. However, a critical flaw in the setPassword function permits any user to change the stored password, severely compromising the contract's integrity and security.

Vulnerability Details

The setPassword function is intended to be restricted for use by the contract owner only. Nevertheless, the function lacks any access control mechanisms, such as the commonly used onlyOwner modifier, to enforce this restriction. Consequently, any malicious actor can call this function to change the password to a value of their choosing.

Code Snippet:

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

Impact

This vulnerability carries a high severity rating. Unauthorized users can change the password, potentially leading to unauthorized access or other malicious activities, if the password is used for critical operations within or outside the blockchain environment.

POC (Proof of Concept)

function test_anyone_can_set_password() public{
vm.startPrank(address(2));
string memory new_passwd = "weeeeeee";
passwordStore.setPassword(new_passwd);
vm.stopPrank();
vm.startPrank(owner);
string memory get_passwd = passwordStore.getPassword();
//console.log(get_passwd);
assertEq(new_passwd,get_passwd);
}

Add the POC function above to the PasswordStore.t.sol file

Output:

Running 1 test for test/PasswordStore.t.sol:PasswordStoreTest
[PASS] test_anyone_can_set_password() (gas: 22887)
Traces:
[22887] PasswordStoreTest::test_anyone_can_set_password()
├─ [0] VM::startPrank(0x0000000000000000000000000000000000000002)
│ └─ ← ()
├─ [6686] PasswordStore::setPassword(weeeeeee)
│ ├─ emit SetNetPassword()
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [0] VM::startPrank(0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38)
│ └─ ← ()
├─ [3320] PasswordStore::getPassword() [staticcall]
│ └─ ← weeeeeee
└─ ← ()

we an see that the password has changed.

Tools Used

  • Foundry

  • Manual Review

Recommendations

  • Implement the onlyOwner modifier on the setPassword function to ensure that only the contract owner can change the password. The onlyOwner modifier should verify that msg.sender is equal to the s_owner before proceeding with the function execution.

modifier onlyOwner() {
require(msg.sender == s_owner, "Not authorized");
_;
}
​
function setPassword(string memory newPassword) external onlyOwner {
s_password = newPassword;
emit SetNetPassword();
}
Updates

Lead Judging Commences

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