Summary

function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}

Function setPassword allows any caller from outside of the contract to change the s_password variable that represents the password of the owner. The function does not check for the msg.sender to be the owner that deployed the contract originally.

Vulnerability Details

The setPassword function does not use any modifier nor check, that would ensure that the caller of the function is the address that deployed the contract. Since the function is only marked as "external", the function is accessible to anyone interacting with the contract. The function can be called with the specified string parameter causing a change of the password stored in the contract.

Impact

This should be considered a high-risk vulnerability as the contract is supposed to allow only the owner/deployer of the contract to change the password. This vulnerability allows any caller to change the password stored in the contract and can cause the deployer to lose the stored variable/password.

Tools Used

Contract analysis and local testing

Recommendations

I would recommend using onlyOwner modifier for both functions (setPassword, getPassword) to ensure only the deployer of the contract can access and modify the value itself.

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

Alternatively, I would propose using the same check as in the getPassword function that does prevent this misuse. However, the first proposal should be more gas-efficient and I would strongly recommend its implementation.