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

Missing Ownership Verification in setPassword Function

charalab0ts

Summary

The PasswordStore :: setPassword function lack of an ownership check

Vulnerability Details

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

This function is designed to permit only the contract's owner to establish a new password. However, the absence of access control within this function fails to verify if the caller is indeed the owner.

Consequently, any address can invoke this function and modify the password.

function test_anyone_can_set_password() public {
vm.startPrank(address(1)); // Simulate a call from a non-owner address
string memory newPassword = "hackedPassword";
passwordStore.setPassword(newPassword); // This should not be allowed, but the contract doesn't check for ownership
vm.startPrank(owner); // Switch back to the owner
string memory actualPassword = passwordStore.getPassword(); // Get the password
assertEq(actualPassword, newPassword); // The password should have been changed by the non-owner
}

This test will pass, demonstrating the security flaw

Impact

this vulnerability could lead to a complete loss of control over the contract for the legitimate owner, unauthorized access to contract-controlled resources, and exposure of sensitive data.

Tools Used

-Foundry
-Manual Review

Recommendations

You should add a modifier that checks if the message sender is the owner of the contract. If not, the function should revert. Here's how you can implement this:

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

This way, only the owner of the contract can set a new password, preventing unauthorized access.
You can also use this modifier to the PasswordStore::getPassword function

- function getPassword() external view returns (string memory) {
- if (msg.sender != s_owner) {
- revert PasswordStore__NotOwner();
- }
- return s_password;
- }
​
+ function getPassword() external view onlyOwner returns (string memory) {
+ return s_password;
+ }
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.