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

Unauthorized Password Modification in 'setPassword' Function

Chandr

Summary

The audit of the "PasswordStore" smart contract reveals a High security issue in the setPassword function. The function is intended to allow the owner of the contract to set or update a password. However, it fails to include a crucial ownership check before performing the password update. This omission allows any address to call the function and change the password, effectively bypassing the intended access control and compromising the confidentiality of the stored data.

Vulnerability Details

The vulnerability resides in the setPassword function. The function is designed to update the s_password private variable, which is intended to hold a secure and private password set by the owner of the contract. However, upon examining the function, it's evident that there's no check to ensure that the msg.sender (the entity calling the function) is indeed the owner of the contract (stored in the s_owner private variable).

This lack of verification means that any external entity or contract interacting with the "PasswordStore" contract can call setPassword and overwrite the s_password variable, thus changing the password. The correct approach would have been to include a modifier or a requirement check to ensure that msg.sender is equal to s_owner before allowing any changes to s_password.

POC

add this code to: PasswordStore.t.sol

function test_non_owner_can_set_password() public {
// Set initial password by the owner
vm.startPrank(owner);
string memory initialPassword = "initialPassword";
passwordStore.setPassword(initialPassword);
vm.stopPrank();
​
// Attempt to change password from a non-owner address
vm.startPrank(address(1)); // address(1) is typically a non-owner address
string memory newUnauthPassword = "unauthorizedPassword";
passwordStore.setPassword(newUnauthPassword); // This shouldn't be allowed
vm.stopPrank();
​
// Revert to owner and check if the password was changed
vm.startPrank(owner);
string memory actualPassword = passwordStore.getPassword();
vm.stopPrank();
// The actual password should still be the initial one if the contract was secure
assertNotEq(actualPassword, newUnauthPassword, "Non-owner could set a new password!");
}

Impact

The impact of this vulnerability is critical. In its current state, the contract fails to fulfill its primary use case, which is to securely store a private password that only the owner can set and retrieve. The security flaw allows an attacker to change the password arbitrarily, potentially locking the legitimate owner out of any system or mechanism that relies on this password. Furthermore, it breaches the trust model of the contract, as the password is no longer confidential or protected from unauthorized changes.

Tools Used

The audit was conducted manually by reviewing the Solidity contract code, identifying security best practices, and analyzing the contract's behavior and access control mechanisms.

Recommendations

To mitigate this vulnerability, it is imperative to enforce strict access control for the setPassword function. Below are the steps to correct the current security flaw:

Implement an "onlyOwner" modifier that checks whether the msg.sender is the s_owner before executing the function's logic. This modifier should be used in the setPassword function to ensure that only the owner can change the password.

modifier onlyOwner() {
require(msg.sender == s_owner, "PasswordStore: Caller is not the owner");
_;
}

Apply the "onlyOwner" modifier to the setPassword function:

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

Additionally, consider implementing similar ownership checks for other sensitive functions within the contract, and ensure that event names correctly reflect their purpose (e.g., SetNewPassword instead of SetNetPassword).
It's also recommended to follow testing practices, including writing automated tests that check the ownership logic and other critical aspects of the smart contract, to prevent similar issues in the future.
By applying these changes, the contract will uphold the principle of data confidentiality and ensure that only authorized addresses (the owner) can make critical changes to the contract's state.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year 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.