Codehawks First Flight-1 - PasswordStore
This is my Audit report for the Codehawks First Flight #1: PasswordStore. Although it’s a bit late to be auditing this contract, first flights are one of the best ways to dip one’s toes in the vast ocean of Smart Contract Audits. This being the first one in the series is an obvious choice for a staring point.
Happy reading!
Getting started
This particular contest involves only a single Smart Contract PasswordStore.sol. PasswordStore is a smart contract application for storing a password. Users should be able to store a password and then retrieve it later. Others should not be able to access the password.
Find the Contract below -
// 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;
string private s_password;
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.
*/
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;
}
}
Issues
High -
1. The `setPassword()` function does not use any access controls.
Impact - The setPassword()
function does not use any access controls and can be called by anyone to change the password.
Mitigation - Modifiers or other access control methods should be used to restrict access to functions handling critical functionality.
function setPassword(string memory newPassword) external {
if (msg.sender != s_owner) {
revert PasswordStore__NotOwner();
}
s_password = newPassword;
emit SetNetPassword();
}
2. The value of the `s_password` private variable is not actually private
Impact - Private information should never be stored on chain since nothing stored on chain is actually private. Any sensitive information stored in a state variable (even if it is private) can be easily retrieved by anyone.
Mitigation - Storing sensitive information on chain should always be avoided since everything on chain is public. Sensitive information should alwyas be stored off-chain.