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

s_owner is equal to sender not checked before setting a password

editdev

Summary

In the contract, s_owner is set to msg.sender in the constructor. This means that the address that deploys the contract becomes the owner. In the setPassword function, there's no require statement that checks if msg.sender is equal to the owner before setting a password.

Vulnerability Details

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

If the is no check that the sender is equal to the owner it leaves a huge risk, If it is not checked, this gives access to a bad actor or anyone who is not s_owner to exploit it. Using a require statement to check if the sender is equal to the owner makes it more secure and less vulnerable to attacks.

Impact

Here's an example of a contract without the require statement:

pragma solidity ^0.8.0;
​
contract Storage {
address private owner;
uint256 public data;
​
constructor() public {
owner = msg.sender;
}
​
function set(uint256 _data) public {
// Missing require statement to check if msg.sender is the owner
data = _data;
}
}

In this contract, any address can call the set function, not just the owner. This could be a problem if the data is sensitive and should only be modified by the owner.

Now, let's write a test case in JavaScript to show this:

const Storage = artifacts.require('Storage');
​
contract('Storage', (accounts) => {
let storage = null;
const owner = accounts[0];
const nonOwner = accounts[1];
​
before(async () => {
storage = await Storage.deployed();
});
​
it('Non-owner can change the data', async () => {
const newData = 20;
await storage.set(newData, { from: nonOwner });
const result = await storage.data.call();
assert(result.toNumber() === newData);
});
});

In this test case, we're using a non-owner account to call the set function. The set function changes the data state variable in the contract. The test passes, showing that a non-owner can change the data, thus exploiting the contract.

Tools Used

Mocha, Chai, Foundry

Recommendations

Check that the sender is equal to the owner of the contract.

+ require(msg.sender == s_owner);
s_password = newPassword;
Updates

Lead Judging Commences

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!