MrjoryStewartBaxter - Missing delegateCall function when performing liquidations.

[sherlock-admin]

MrjoryStewartBaxter

high

Missing delegateCall function when performing liquidations.

Summary

When trying to execute the liquidateVaultsForToken function, it will be reverted because the required delegateCall is missing (Sequence diagram from README confirms this issue, as function should be in place). Particularly when making a call to the asyncSwapper for performing the swapping operation.

Vulnerability Detail

Inside the _performLiquidation() function within the LiquidationRow.sol contract there is a piece of code which uses the swap function to call the swapper, which executes the corresponding swapping using the assets available in the LiquidationRow.sol contract.

However, as there is no delegateCall function, when we invoke the swap() a revert will follow. Because of this issue, all rewards claimed from destination vaults are trapped within the contract.

// Line under _performLiquidation()

https://github.com/sherlock-audit/2023-06-tokemak/blob/5d8e902ce33981a6506b1b5fb979a084602c6c9a/v2-core-audit-2023-07-14/src/liquidation/LiquidationRow.sol#L251

uint256 amountReceived = IAsyncSwapper(asyncSwapper).swap(params);

POC

Using the following test test_OnlyLiquidateGivenTokenForGivenVaults() inside the LiquidiationRow.t.sol :

function test_OnlyLiquidateGivenTokenForGivenVaults() public {
         BaseAsyncSwapper asyncSwapper = new BaseAsyncSwapper(ZERO_EX_MAINNET);
        
        liquidationRow.addToWhitelist(address(asyncSwapper));

        _mockComplexScenario(address(testVault));
        IDestinationVault[] memory vaults = _initArrayOfOneTestVault();
        liquidationRow.claimsVaultRewards(vaults);

        SwapParams memory swapParams =
            SwapParams(address(rewardToken2), 200, address(targetToken), 200, new bytes(0), new bytes(0));
        vm.expectRevert();
        liquidationRow.liquidateVaultsForToken(address(rewardToken2), address(asyncSwapper), vaults, swapParams);          


        assertTrue(liquidationRow.balanceOf(address(rewardToken), address(testVault)) == 100);
        assertTrue(liquidationRow.balanceOf(address(rewardToken2), address(testVault)) == 0);
        assertTrue(liquidationRow.balanceOf(address(rewardToken3), address(testVault)) == 100);
        assertTrue(liquidationRow.balanceOf(address(rewardToken4), address(testVault)) == 100);
        assertTrue(liquidationRow.balanceOf(address(rewardToken5), address(testVault)) == 100);

        assertTrue(liquidationRow.totalBalanceOf(address(rewardToken)) == 100);
        assertTrue(liquidationRow.totalBalanceOf(address(rewardToken2)) == 0);
        assertTrue(liquidationRow.totalBalanceOf(address(rewardToken3)) == 100);
        assertTrue(liquidationRow.totalBalanceOf(address(rewardToken4)) == 100);
        assertTrue(liquidationRow.totalBalanceOf(address(rewardToken5)) == 100);
    }

It will revert, since the function is not working correctly due to the aforementioned missing delegateCall.

 │   └─ ← ()
    ├─ [14199] liquidationRow::liquidateVaultsForToken(rewardToken2: [0x2a9e8fa175F45b235efDdD97d2727741EF4Eee63], BaseAsyncSwapper: [0x7926BcF47Dd54194DA501a435CfbC210Ad083A64], [0xA05BC0EA7A36BCAD8416749af8A630a891e2D46C], (0x2a9e8fa175F45b235efDdD97d2727741EF4Eee63, 200, 0x4d04375eCD86c2B81eb0F55B37aA3fAb41CeCBc4, 200, 0x, 0x)) 
    │   ├─ [651] AccessController::hasRole(0x5e17fc5225d4a099df75359ce1f405503ca79498a8dc46a7d583235a0ee45c16, LiquidateVaultsForToken: [0x34A1D3fff3958843C43aD80F30b94c510645C316]) [staticcall]
    │   │   └─ ← true
    │   ├─ [4866] BaseAsyncSwapper::swap((0x2a9e8fa175F45b235efDdD97d2727741EF4Eee63, 200, 0x4d04375eCD86c2B81eb0F55B37aA3fAb41CeCBc4, 200, 0x, 0x)) 
    │   │   ├─ [2563] rewardToken2::balanceOf(BaseAsyncSwapper: [0x7926BcF47Dd54194DA501a435CfbC210Ad083A64]) [staticcall]
    │   │   │   └─ ← 0
    │   │   └─ ← "InsufficientBalance(0, 200)"

Impact

The rewards meant for the destination's rewarders within the LiquidationRow contract will never be distributed and will be stuck inside that contract .

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/5d8e902ce33981a6506b1b5fb979a084602c6c9a/v2-core-audit-2023-07-14/src/liquidation/LiquidationRow.sol#L251

Tool used

Manual Review

Recommendation

delegateCall function should be implemented when calling asyncSwapper.

Duplicate of #205