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

Any user can set a password using setPassword, which means the owner loses control over his password

setstacklist

Summary

The PasswordStore::setPassword(string memory newPassword) method does not check for the owner, so any user can set a password.

Vulnerability Details

contract PasswordStore {
@> function setPassword(string memory newPassword) external {
s_password = newPassword;
emit SetNetPassword();
}
}

The owner may lose his password he set using this contract because the PasswordStore::setPassword(string memory newPassword) method does not check it is called by the owner. Any user can overwrite the password at any time. This way the owner will lose access to his password. Owner may lead to loss of access to the service for which the password was saved.

Impact

Loss of owner control over the password. Any user can set a password. Thus, when the owner wants to get a password to log into some service, he will not be able to do this, since another user has changed the password on the storage.

Tools Used

No specific tools.

Recommendations

It is necessary to add a check for the owner before setting a password. This can be done in several ways:

  1. Add a revert, as done in the PasswordStore::getPassword() method.

function setPassword(string memory newPassword) external {
+ if (msg.sender != s_owner) {
+ revert PasswordStore__NotOwner();
+ }
+
s_password = newPassword;
emit SetNetPassword();
}

  1. Create an onlyOwner modifier and apply it to PasswordStore::setPassword(string memory newPassword) and PasswordStore::getPassword() to check msg.sender is the owner's address.

+ modifier onlyOwner() {
+ if (msg.sender != s_owner) {
+ revert PasswordStore__NotOwner();
+ }
+ _;
+ }
​
- function setPassword(string memory newPassword) external {
+ function setPassword(string memory newPassword) external onlyOwner {
s_password = newPassword;
emit SetNetPassword();
}
​
- function getPassword() external view returns (string memory) {
+ function getPassword() external view returns (string memory) onlyOwner {
- if (msg.sender != s_owner) {
- revert PasswordStore__NotOwner();
- }
return s_password;
}

  1. Use Ownable from OpenZeppelin.

+ import "@openzeppelin/contracts/access/Ownable.sol";
​
- contract PasswordStore {
+ contract PasswordStore is Ownable {
- function setPassword(string memory newPassword) external {
+ function setPassword(string memory newPassword) external onlyOwner {
s_password = newPassword;
emit SetNetPassword();
}
​
- function getPassword() external view returns (string memory) {
+ function getPassword() external view returns (string memory) onlyOwner {
- if (msg.sender != s_owner) {
- revert PasswordStore__NotOwner();
- }
return s_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-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.