Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Lack of checking the caller of setPassword results in non-owner able to change password

Summary

The setPassword function in PasswordStore contract allows a non-owner to change the password, which should not be allowed as per requirements.

Vulnerability Details

The setPassword function does not check to see if the caller (msg.sender) is equal to the owner (PasswordStore:s_owner)

Impact

As a contract to be deployed to a public production blockchain, the likelihood of this vulnerability being exploited is high. Additionally, the impact upon being exploited would be high as control over managing the sensitive content (s_password) would be lost. Therefore, overall impact is high.

Two Javascript tests that highlight this problem can be found at the GitHub links below...
non-owner cannot set password
setPassword called by non-owner should not emit SetNetPassword event

Here is an image showing these two tests failing.

Tools Used

Visual Studio Code
Hardhat

Recommendations

A modifier, onlyOwner, should be created as shown below.

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

This modifier should be added to the setPassword function which will prevent a non-owner from being able to execute the function.

function setPassword(string memory newPassword) external onlyOwner {
s_password = newPassword;
emit SetNetPassword();
}
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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!