sherlock-audit / 2024-04-interest-rate-model-judging

2 stars 1 forks source link

Squilliam - [M-3] `EscrowedExa::initialize` can be frontrun due to lack of access control, allowing attackers to grant themselves admin role and manipulate key parameters such as vesting periods and reserve ratio #18

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

Squilliam

medium

[M-3] EscrowedExa::initialize can be frontrun due to lack of access control, allowing attackers to grant themselves admin role and manipulate key parameters such as vesting periods and reserve ratio

Summary

The initialize function in the EscrowedExa::initialize contract is vulnerable to frontrunning attacks due to lack of access control , allowing attackers to grant themselves admin control and manipulate key parameters such as vesting periods and reserve ratio with the power of an admin.

Vulnerability Detail

function initialize(uint40 vestingPeriod_, uint256 reserveRatio_) external initializer {
    __ERC20_init("escrowed EXA", "esEXA");
    __ERC20Permit_init("escrowed EXA");
    __ERC20Votes_init();
    __AccessControl_init();

    _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
    /// @dev address(0) holds the TRANSFERRER_ROLE so the token can be minted or burnt.
    _grantRole(TRANSFERRER_ROLE, address(0));

    setVestingPeriod(vestingPeriod_);
    setReserveRatio(reserveRatio_);
    exa.safeApprove(address(sablier), type(uint256).max);
  }

The code above can be found in ExcrowedExa::initialize and if exactly protocol deploys their contracts and does not identify that they have already been initialized, users could start using a system that was compromised from the start. Attackers could grant themselves admin role, set vesting periods, and set reserve ratio.

If an attacker sees this transaction in the mempool, they can frontrun the transaction with a higher gas pay and call the initialize function before the owner. This would be possible because the initialize function is not protected against frontrunning in the transaction ordering sense. The initializer modifier in Solidity is specifically designed to ensure that a function marked with it can only be invoked once during the contract's initialization phase. The initializer modifier does not provide access control functionality.

Impact

If attackers exploit this vulnerability by granting themselves admin roles, they gain unrestricted control over the system. They could manipulate key parameters such as vesting periods and reserve ratios, which could lead to unfair distribution of tokens, financial losses for users, and a significant loss of trust in the system. This could lead to the deployment of compromised contracts, which users may start using without realizing that the system has been compromised from the start and could cause severe financial loss for the entire protocol and all its users.

Code Snippet

This vulnerability can be found here: https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/periphery/EscrowedEXA.sol?plain=1#L43-L56

Tool used

Manual Review

Recommendation

Implement valid access control on the initialize() to ensure only the relevant deployer can initialize such as an onlyOwner modifier or automatically call initialize in your deploy function in your setup.

santipu03 commented 2 months ago

This issue is marked as invalid according to the Sherlock guidelines:

Front-running initializers: Front-running initializers where there is no irreversible damage or loss of funds & the protocol could just redeploy and initialize again is not a valid issue.