c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/FeeSplitter.sol#L80-L87 https://github.com/code-423n4/2024-01-curves/blob/516aedb7b9a8d341d0d2666c23780d2bd8a9a600/contracts/FeeSplitter.sol#L103-L117

Vulnerability details

Impact

The use of transfer() is deprecated as the transaction may fail in numerous scenarios:

The claimer smart contract does not implement a payable function, or it implements a payable fallback function that consumes more than 2300 gas units.
The claimer smart contract implements a payable fallback function requiring less than 2300 gas units, but it is called through a proxy, which increases the call's gas consumption above 2300 units.

For instance, most multisignature wallet overheads are at least 2300 gas units.

The rationale behind this deprecation is that using a fixed amount of gas for an action shows poor resilience for the future of the ecosystem, considering the changes in the EVM and smart contract behaviors.

In the codebase, you cannot transfer fees, and the fees are linked to the holders who held Curve tokens at the time of the fees. If the fee receiver falls into the above scenarios, the fees will be lost.

Recommended Mitigation Steps

Use call() instead of transfer() in the claimFees function of the FeeSplitter.sol contract.

function claimFees(address token) external {
   updateFeeCredit(token, msg.sender);
   uint256 claimable = getClaimableFees(token, msg.sender);
   if (claimable == 0) revert NoFeesToClaim();
   tokensData[token].unclaimedFees[msg.sender] = 0;
-  payable(msg.sender).transfer(claimable);
+  (bool success, ) = msg.sender.call{value: claimable}("");
+  require(success, "Transfer failed.");
   emit FeesClaimed(token, msg.sender, claimable);
}

Use call() instead of transfer() in the batchClaiming function of the FeeSplitter.sol contract.

function batchClaiming(address[] calldata tokenList) external {
        uint256 totalClaimable = 0;
        for (uint256 i = 0; i < tokenList.length; i++) {
            address token = tokenList[i];
            updateFeeCredit(token, msg.sender);
            uint256 claimable = getClaimableFees(token, msg.sender);
            if (claimable > 0) {
                tokensData[token].unclaimedFees[msg.sender] = 0;
                totalClaimable += claimable;
                emit FeesClaimed(token, msg.sender, claimable);
            }
        }
        if (totalClaimable == 0) revert NoFeesToClaim();
-       payable(msg.sender).transfer(totalClaimable);
+       (bool success, ) = msg.sender.call{value: totalClaimable}("");
+       require(success, "Transfer failed.");
    }