Lines of code

Vulnerability details

Impact

If more than one pool has underlying = address(0) then RewardHandler.burnFees() will fail or use ETH balance from FeeBurner.sol.

Proof of Concept

RewardHandler.sol#L40-L50

uint256 ethBalance = address(this).balance;
        address[] memory tokens = new address[](pools.length);
        for (uint256 i; i < pools.length; i = i.uncheckedInc()) {
            ILiquidityPool pool = ILiquidityPool(pools[i]);
            address underlying = pool.getUnderlying();
            if (underlying != address(0)) {
                _approve(underlying, address(feeBurner));
            }
            tokens[i] = underlying;
        } 
        feeBurner.burnToTarget{value: ethBalance}(tokens, targetLpToken);

FeeBurner.sol#L56-L65

for (uint256 i; i < tokens_.length; i = i.uncheckedInc()) {
            IERC20 token_ = IERC20(tokens_[i]);

            // Handling ETH
            if (address(token_) == address(0)) {
                if (msg.value == 0) continue;
                burningEth_ = true;
                swapperRouter_.swapAll{value: msg.value}(address(token_), _WETH);
                continue;
            }

• RewardHandler.burnFees() calls feeBurner.burnToTarget() with its entire ethBalance.
• If two pools have underlying = address(0) then feeBurner.burnToTarget() will try to swap the entire ethBalance from RewardHandler twice.
• The call will either revert or use FeeBurners own balance.

Tools Used

Manual Review

Recommended Mitigation Steps

Don’t loop over using the same msg.value when dealing with multiple pools using underlying = address(0).
Instead make the swap based on an individual per token basis.

The text was updated successfully, but these errors were encountered:

All reactions