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

Insecure and explicit password

0xnhattranduy

Summary

The functions of the PasswordStore.sol contract are to manage a password that can be only set and read by the contract owner. But through examining the code as well as using some tricks exploiting the nature of Solidity, one can easily manage to crack this rule.

Vulnerability Details

  1. Not just owner but everyone can call the contract to set the password

This is because this function

/*
* @notice This function allows only the owner to set a new password.
* @param newPassword The new password to set.
*/
function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}

doesn't check whether the sender is owner or not.

This bug can be demonstrated through this test, here, we call the setPassword function from a non-owner address and expect it to revert, but it fails.

function test_non_owner_setting_password_reverts() public {
vm.startPrank(address(1));
​
vm.expectRevert(PasswordStore.PasswordStore__NotOwner.selector);
passwordStore.setPassword("random password");
}

  1. Not just owner but everyone can read the password

We have many ways to read the password stored in the contract, 2 possible ways are:

  • Read the storage slot where the password variable is placed:

const passwordBytes = await ethers.provider.getStorage(passwordStore.target, 1);
let expectedPassword = ethers.toUtf8String(passwordBytes);
const firstNullIndex = expectedPassword.indexOf("\0");
if (firstNullIndex > - 1)
expectedPassword = expectedPassword.substring(0, firstNullIndex)
expect(expectedPassword).to.equal(password);

  • Trace the password value set by the owner in the past

const ABI = ... // contract abi
const iface = new ethers.Interface(ABI)
const tx = await ethers.provider.getTransaction(txHash);
const decodeData = iface.parseTransaction({data: tx?.data!, value: tx?.value})
expect(decodeData?.args[0]).to.be.eq(password)

The above codes can be found here

Impact

  • These bugs can cause severe consequences to the owner. If he use the given password to lock his assets or personal data, he will lose everything.

Tools Used

  • Foundry: to demonstrate the password manipulation bug

  • Hardhat: to read the on-chain password using ethers.js client

Recommendations

  1. To restrict who can modify the password, we should add this requirement on top of the setPassword function, so no one but the owner can call it

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

  1. Basically, we shouldn't store anything that is meant to be private on-chain like 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-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.