Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Admin of the `RockPaperScissors` contract can be changed by the current admin

nebosvod

Description: The RockPaperScissors::setAdmin function can currently only be called by the admin. This invalidates the docs which state that the owner of the RockPaperScissors should be able to change the admin.

function setAdmin(address _newAdmin) external {
require(msg.sender == adminAddress, "Only admin can set new admin");
require(_newAdmin != address(0), "Admin cannot be zero address");
​
adminAddress = _newAdmin;
}

Impact: The RockPaperScissors contract owner has no power over the contract admin once they've set the first admin. This gives all power to the admin to change and set new admins. This is dangerous because the admin is the one who can withdraw the fees from the contract. Therefore, the contract owner has no control over the contract administration whatsoever.

Recommended Mitigation:
There are a different ways to mitigate this:

  1. Use the Ownable abstract class by openzeppelin by extending the RockPaperScissors contract which will make the initial deployer of the contract the owner and use the onlyOwner modifier for the setAdmin function to prevent the admin and anyone else from setting a new admin.

+ import "@openzeppelin/contracts/access/Ownable.sol";
+ contract RockPaperScissors is Ownable {
- constructor() {
+ constructor() Ownable (msg.sender) {
winningToken = new WinningToken();
adminAddress = msg.sender;
}
}
- function setAdmin(address _newAdmin) external {
+ function setAdmin(address _newAdmin) external onlyOwner {
- require(msg.sender == adminAddress, "Only admin can set new admin");
require(_newAdmin != address(0), "Admin cannot be zero address");
adminAddress = _newAdmin;
}

  1. Introduce a contractOwner property and change the require check in the setAdmin function to verify it's called by the contract owner. This will ensure only the contract owner has the means to set and change the admin any time.

+ address public owner;
​
constructor() {
winningToken = new WinningToken();
+ owner = msg.sender;
adminAddress = msg.sender;
}
​
- function setAdmin(address _newAdmin) external {
- require(msg.sender == adminAddress, "Only admin can set new admin");
+ require(msg.sender == owner, "Only admin can set new admin");
require(_newAdmin != address(0), "Admin cannot be zero address");
​
adminAddress = _newAdmin;
}
​
Updates

Appeal created

m3dython Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational

Code suggestions or observations that do not pose a direct security risk.

Support

FAQs

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