code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

No Slippage Protection During Redeeming Matured Positions #157

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L385

Vulnerability details

Proof-of-Concept

The lack of slippage checks can cause redemption of matured positions to happen at a bad rate/prices, resulting the trade to receive fewer tokens than the fair market rate/price.

The NotionalTradeModule._redeemMaturedPositions function calls the _redeemFCashPosition(_setToken, fCashPosition, receiveToken, fCashBalance, 0); which set the _minReceiveAmount parameter to 0. Thus, this effectively disable the slippage control.

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/index-coop-notional-trade-module/contracts/protocol/modules/v1/NotionalTradeModule.sol#L385

function _redeemMaturedPositions(ISetToken _setToken)
internal
{
    ISetToken.Position[] memory positions = _setToken.getPositions();
    uint positionsLength = positions.length;

    bool toUnderlying = redeemToUnderlying[_setToken];

    for(uint256 i = 0; i < positionsLength; i++) {
        // Check that the given position is an equity position
        if(positions[i].unit > 0) {
            address component = positions[i].component;
            if(_isWrappedFCash(component)) {
                IWrappedfCashComplete fCashPosition = IWrappedfCashComplete(component);
                if(fCashPosition.hasMatured()) {
                    (IERC20 receiveToken,) = fCashPosition.getToken(toUnderlying);
                    if(address(receiveToken) == ETH_ADDRESS) {
                        receiveToken = weth;
                    }
                    uint256 fCashBalance = fCashPosition.balanceOf(address(_setToken));
                    _redeemFCashPosition(_setToken, fCashPosition, receiveToken, fCashBalance, 0);
                }
            }
        }
    }
}

Recommended Mitigation Steps

Consider adding a pre-defined slippage rate (e.g. 5%) so that if the slippage rate falls beyond the acceptable rate, the transaction will revert. A settler and getter functions can be defined for configuring the pre-defined slippage rate.

ckoopmann commented 2 years ago

Duplicate of: https://github.com/code-423n4/2022-06-notional-coop-findings/issues/37