Missing zero approval

Summary

Few tokens require the approval limit to be 0 before setting a new approval limit like USDT. If trade.sellToken is one of such tokens then the approval will fail causing the trade to fail

Vulnerability Detail

Trade is executed using _executeInternal function

function _executeInternal(
        Trade memory trade,
        uint16 dexId,
        address spender,
        address target,
        uint256 msgValue,
        bytes memory executionData
    ) internal returns (uint256 amountSold, uint256 amountBought) {
...
if (spender != Deployments.ETH_ADDRESS && DexId(dexId) != DexId.NOTIONAL_VAULT) {
            _approve(trade, spender);
        }
...
}

Now _approve function is called

function _approve(Trade memory trade, address spender) private {
        uint256 allowance = _isExactIn(trade) ? trade.amount : trade.limit;
        IERC20(trade.sellToken).checkApprove(spender, allowance);
    }

Now checkApprove function simply sets the approval of spender to "allowance"
As we can see this is missing 0 approval limit before setting the allowance to passed "allowance"

Impact

Trade will fail due to missing zero approval.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol#L113

Tool used

Manual Review

Recommendation

Approve 0 amount before setting the actual approval limit

function _approve(Trade memory trade, address spender) private {
        uint256 allowance = _isExactIn(trade) ? trade.amount : trade.limit;
IERC20(trade.sellToken).checkApprove(spender, 0);
        IERC20(trade.sellToken).checkApprove(spender, allowance);
    }

Duplicate of #59

sherlock-audit / 2022-09-notional-judging

csanuragjain - Missing zero approval #109