code-423n4 / 2024-01-init-capital-invitational-findings

1 stars 0 forks source link

SwapType.CloseExactOut balance check too strict can be DOSed #16

Open c4-bot-3 opened 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-init-capital-invitational/blob/a01c4de620be98f9e57a60cf6a82d4feaec54f58/contracts/hook/MarginTradingHook.sol#L422

Vulnerability details

Impact

SwapType.CloseExactOut balance check too strict can be DOSed

Proof of Concept

In CoreCallback function, we have the logic below

uint amtOut = IERC20(swapInfo.tokenOut).balanceOf(address(this));
if (swapInfo.swapType == SwapType.OpenExactIn) {
    // transfer to coll pool to mint
    IERC20(swapInfo.tokenOut).safeTransfer(marginPos.collPool, amtOut);
    emit SwapToIncreasePos(swapInfo.initPosId, swapInfo.tokenIn, swapInfo.tokenOut, amtIn, amtOut);
} else {
    // transfer to borr pool to repay
    uint amtSwapped = amtIn;
    if (swapInfo.swapType == SwapType.CloseExactOut) {
        _require(IERC20(swapInfo.tokenOut).balanceOf(address(this)) == swapInfo.amtOut, Errors.SLIPPAGE_CONTROL);
        amtSwapped -= IERC20(swapInfo.tokenIn).balanceOf(address(this));
    }
    emit SwapToReducePos(swapInfo.initPosId, swapInfo.tokenIn, swapInfo.tokenOut, amtSwapped, amtOut);
}

note the logic

    if (swapInfo.swapType == SwapType.CloseExactOut) {
        _require(IERC20(swapInfo.tokenOut).balanceOf(address(this)) == swapInfo.amtOut, Errors.SLIPPAGE_CONTROL);
        amtSwapped -= IERC20(swapInfo.tokenIn).balanceOf(address(this));
    }

this check

_require(IERC20(swapInfo.tokenOut).balanceOf(address(this)) == swapInfo.amtOut, Errors.SLIPPAGE_CONTROL);

is too strict, malicious uer can DOS the swap by simply perform a tiny swap by frontruning the swap to change the token ratio in the LP pool

for example, the swapInfo.amtOut is 1000 USDC, and swapInfo.tokenOut is USDC

it is difficult to get exact 1000 USDC,

any swap that executes before the swap can make the received USDC amount to 1000.1 USDC or 999. USDC.

then swap transaction revert.

Tools Used

Manual review

Recommended Mitigation Steps

do not use ==, use >=

Assessed type

DoS

hansfriese commented 8 months ago

Downgrade to Medium as there is no direct fund loss.

c4-judge commented 8 months ago

hansfriese changed the severity to 2 (Med Risk)

c4-judge commented 8 months ago

hansfriese marked the issue as primary issue

c4-sponsor commented 8 months ago

fez-init (sponsor) confirmed

c4-judge commented 8 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 8 months ago

hansfriese marked the issue as selected for report