search

sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

KupiaSec - Setting `dsId` to an expired `DS` will lead to the reversal of the `FlashSwapRouter.swapDsforRa()` function #232

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

KupiaSec

Medium

Setting dsId to an expired DS will lead to the reversal of the FlashSwapRouter.swapDsforRa() function

Summary

There is an exchange of CT + DS to RA during the DS selling process. This exchange can only be performed with the currently activated DS. Therefore, setting dsId to an expired DS will lead to the reversal of the selling.

Vulnerability Detail

The FlashSwapRouter.swapDsforRa() function has a parameter dsId (line 251) that represents the selling DS.

    function swapDsforRa(
        Id reserveId,
251     uint256 dsId,
        uint256 amount,
        uint256 amountOutMin,
        bytes memory rawDsPermitSig,
        uint256 deadline
    ) external returns (uint256 amountOut) {
        ...
    }

The function __afterFlashswapSell() is invoked during the selling process. As you can see at line 394, it exchanges CT + DS for RA. This exchange can only be performed with the currently activated DS, as indicated in the PsmLib.redeemRaWithCtDs() function. As a result, setting dsId to an expired DS will lead to the reversal of the transaction.

    function __afterFlashswapSell(
        ReserveState storage self,
        uint256 ctAmount,
        Id reserveId,
        uint256 dsId,
        address caller,
        uint256 raAttributed
    ) internal {
        AssetPair storage assetPair = self.ds[dsId];
        assetPair.ds.approve(owner(), ctAmount);
        assetPair.ct.approve(owner(), ctAmount);

        IPSMcore psm = IPSMcore(owner());

394     (uint256 received,) = psm.redeemRaWithCtDs(reserveId, ctAmount);

        // for rounding error and to satisfy uni v2 liquidity rules(it forces us to repay 1 wei higher to prevent liquidity stealing)
        uint256 repaymentAmount = received - raAttributed;

        Asset ra = assetPair.ra;

        assert(repaymentAmount + raAttributed >= received);

        // send caller their RA
        IERC20(ra).safeTransfer(caller, raAttributed);
        // repay flash loan
        IERC20(ra).safeTransfer(msg.sender, repaymentAmount);
    }

Impact

Selling DS could be reverted.

Code Snippet

https://github.com/sherlock-audit/2024-08-cork-protocol/tree/main/Depeg-swap/contracts/core/flash-swaps/FlashSwapRouter.sol#L249-L265

https://github.com/sherlock-audit/2024-08-cork-protocol/tree/main/Depeg-swap/contracts/core/flash-swaps/FlashSwapRouter.sol#L357-L378

Tool used

Manual Review

Recommendation

Don't include the parameter dsId.

sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Why someone would set dsId to an expired DS?