Lack of Access Control in RewardControl

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/abstract/RewardControl.sol#L92-L109

Vulnerability details

Impact

The RewardControl contract lacks proper access control mechanisms, allowing any user to call critical functions such as withdraw, depositBatch, and withdrawWithSig. This opens the contract to unauthorized actions, including unauthorized withdrawals, disruption of intended functionality, and potential security vulnerabilities. An attacker could exploit this to drain funds from the contract, spam the contract with unnecessary deposits, or front-run legitimate transactions, leading to a loss of funds and disruption of the contract's operations.

Proof of Concept

The following functions in the RewardControl contract are vulnerable due to the lack of access control:

1. withdraw Function:

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/abstract/RewardControl.sol#L92-L95

• This function allows any user to withdraw funds from the contract to any address, without any restrictions.

function withdraw(address to, uint256 amount) external {
    _withdraw(msg.sender, to, amount);
}

2. withdrawWithSig Function:

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/abstract/RewardControl.sol#L96-L106

• This function allows any user to perform a gasless withdrawal using a signature, without any restrictions.

function withdrawWithSig(address from, address to, uint256 amount, uint256 deadline, bytes calldata sig) external {
    // Function implementation
}

Exploit Scenario:

• An attacker could call the withdraw function to withdraw funds from the contract to their own address, even if they are not authorized to do so.
• An attacker could call the depositBatch function with malicious data, such as depositing funds to unintended recipients or spamming the contract with unnecessary deposit.

Tools Used

Manual Code Review

Recommended Mitigation Steps

To mitigate this vulnerability, implement access control mechanisms to restrict who can call critical functions. Here are some recommended steps:

1. Use the Ownable Pattern:

   • Implement the Ownable pattern to restrict access to critical functions to only the contract owner or authorized users.

   import "@openzeppelin/contracts/access/Ownable.sol";
   
   contract RewardControl is IRewards, EIP712, Ownable {
       // ...
   
       function withdraw(address to, uint256 amount) external onlyOwner {
           _withdraw(msg.sender, to, amount);
       }

2. Implement Role-Based Access Control (RBAC):

   • Use OpenZeppelin's AccessControl contract to define roles and restrict access to functions based on roles.

   import "@openzeppelin/contracts/access/AccessControl.sol";
   
   contract RewardControl is IRewards, EIP712, AccessControl {
       bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
   
       constructor() {
           _setupRole(ADMIN_ROLE, msg.sender);
       }
   
       function withdraw(address to, uint256 amount) external onlyRole(ADMIN_ROLE) {
           _withdraw(msg.sender, to, amount);
       }
   
       function withdrawFor(address from, uint256 amount) external onlyRole(ADMIN_ROLE) {
           _withdraw(from, from, amount);
       }
   
       // Other functions can also be restricted using roles
   }

3. Custom Access Control Logic:

   • If more granular control is needed, implement custom access control logic directly in the contract.

   contract RewardControl is IRewards, EIP712 {
       address public admin;
   
       constructor() {
           admin = msg.sender;
       }
   
       modifier onlyAdmin() {
           require(msg.sender == admin, "Not authorized");
           _;
       }
   
       function withdraw(address to, uint256 amount) external onlyAdmin {
           _withdraw(msg.sender, to, amount);
       }
   
       function withdrawFor(address from, uint256 amount) external onlyAdmin {
           _withdraw(from, from, amount);
       }
   
       // Other functions can also be restricted using onlyAdmin
   }

Assessed type

Access Control