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

Missing access control allows anyone to set a new password

Andrew

Summary

The function setPassword doesn't verify msg.sender is the contract owner, which allows any account calling this function to set a new password within the PasswordStore contract.

Vulnerability Details

Access controls limit which accounts can access certain features of smart contracts. In the PasswordStore contract, the setPassword function is missing an access control implementation that requires msg.sender to be s_owner in order to successfully call this function. As a result, any account can call setPassword and change s_password. When getPassword is called, it will return the value of s_password, which can no longer be considered accurate.

Example:

  • Contract owner A deploys a new PasswordStore contract and sets the initial password to "Password" by calling setPassword.

  • Malicious actor B then calls setPassword to overwrite the initial password set by A to a new value of "NewPassword".

  • When A calls getPassword, the retrieved value is "NewPassword" set by B instead of "Password" set by A.

The following foundry test verifies the ability of a non-owner account to modify the value of s_password:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;
​
import {Test, console} from "forge-std/Test.sol";
import {PasswordStore} from "../src/PasswordStore.sol";
import {DeployPasswordStore} from "../script/DeployPasswordStore.s.sol";
​
contract PasswordStoreTest is Test {
PasswordStore public passwordStore;
DeployPasswordStore public deployer;
address public owner;
​
event SetNetPassword();
​
function setUp() public {
deployer = new DeployPasswordStore();
passwordStore = deployer.run();
owner = msg.sender;
}
​
function testNonOwnerCanSetPassword() public {
// Set the initial value of s_password to expectedPassword as the owner
vm.startPrank(owner);
string memory expectedPassword = "Password";
vm.expectEmit();
emit SetNetPassword();
passwordStore.setPassword(expectedPassword);
​
// Verify expectedPassword == s_password
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, expectedPassword);
vm.stopPrank();
​
// Change the value of s_password to wrongPassword from a non-owner account
vm.startPrank(address(1));
string memory wrongPassword = "NewPassword";
vm.expectEmit();
emit SetNetPassword();
passwordStore.setPassword(wrongPassword);
vm.stopPrank();
​
// Verify wrongPassword == s_password
vm.startPrank(owner);
actualPassword = passwordStore.getPassword();
assertEq(actualPassword, wrongPassword);
vm.stopPrank();
}
}

Impact

The first core feature of the PasswordStore contract (storing the password) is vulnerable to manipulation. As any account is able to call setPassword, one must assume the value of s_password is incorrect. As a result, the second core function of the PasswordStore contract (retrieving the password) cannot be called with the expectation of retrieving an accurate value for s_password. The exploitation of setPassword renders getPassword unreliable, resulting in the entire PasswordStore contract being unusable for its intended purpose.

Tools Used

Manual analysis and Foundry (for testing).

Recommendations

Implement an access control that verifies msg.sender is s_owner and reverts using the existing PasswordStore__NotOwner() error if this condition is false, as shown below:

Previous setPassword implementation:

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

New setPassword implementation with access control:

function setPassword(string memory newPassword) external {
if (msg.sender != s_owner) {
revert PasswordStore__NotOwner();
}
s_password = newPassword;
emit SetNetPassword();
}

By adding the following code snippet to setPassword:

if (msg.sender != s_owner) {
revert PasswordStore__NotOwner();
}

the setPassword function will revert if msg.sender is not s_owner, allowing only the contract owner to set s_password.

Updates

Lead Judging Commences

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