code-423n4 / 2022-01-sandclock-findings

0 stars 0 forks source link

No slippage control on _swapUstToUnderlying of NonUSTStrategy.sol #100

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

cccz

Vulnerability details

Impact

There is no slippage control on _swapUstToUnderlying of NonUSTStrategy.sol, which expose strategy to sandwich attack.

And since finishRedeemStable lacks access control, anyone can do a sandwich attack by calling the _swapUstToUnderlying function.

    function finishRedeemStable(uint256 idx) public override(BaseStrategy) {
        super.finishRedeemStable(idx);
        _swapUstToUnderlying();
    }
    ...
    function _swapUstToUnderlying() internal {
        uint256 ustBalance = _getUstBalance();
        if (ustBalance > 0) {
            // slither-disable-next-line unused-return
            curvePool.exchange_underlying(ustI, underlyingI, ustBalance, 0);
        }
    }

Proof of Concept

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/NonUSTStrategy.sol#L107-L110

https://github.com/code-423n4/2022-01-sandclock/blob/main/sandclock/contracts/strategy/NonUSTStrategy.sol#L90-L96

Tools Used

Manual analysis

Recommended Mitigation Steps

Replace 0 in the parameter of exchange_underlying with minout

naps62 commented 2 years ago

duplicate of #8

dmvt commented 2 years ago

duplicate of #7