Grieffer`o.maker` can DOS `initiate()` or `exit()` order if underlying token is ERC777

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L125

Vulnerability details

Impact

If underlying is a ERC777 token msg.sender will not be able to fulfill order if grieffer o.maker decides revert execution on transfer receival.

Proof of Concept

If underlying is a ERC777 token o.maker gets control of execution Safe.transferFrom(uToken, msg.sender, o.maker, a); and is able to revert transaction.

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L125

function initiateVaultFillingZcTokenInitiate(Hash.Order calldata o, uint256 a, Sig.Components calldata c) internal {
    // checks order signature, order cancellation and order expiry
    bytes32 hash = validOrderHash(o, c);

    // checks the side, and the amount compared to available
    uint256 amount = a + filled[hash];

    if (amount > o.premium) { revert Exception(5, amount, o.premium, address(0), address(0)); }

    // TODO cheaper to assign amount here or keep the ADD?
    filled[hash] += a;

    // transfer underlying tokens
    IErc20 uToken = IErc20(o.underlying);
    Safe.transferFrom(uToken, msg.sender, o.maker, a);

    uint256 principalFilled = (a * o.principal) / o.premium;
    Safe.transferFrom(uToken, o.maker, address(this), principalFilled);

    IMarketPlace mPlace = IMarketPlace(marketPlace);
    address cTokenAddr = mPlace.cTokenAddress(o.protocol, o.underlying, o.maturity);

    // perform the actual deposit type transaction, specific to a protocol
    if (!deposit(o.protocol, o.underlying, cTokenAddr, principalFilled)) { revert Exception(6, 0, 0, address(0), address(0)); }

    // alert marketplace
    if (!mPlace.custodialInitiate(o.protocol, o.underlying, o.maturity, o.maker, msg.sender, principalFilled)) { revert Exception(8, 0, 0, address(0), address(0)); }

    // transfer fee in vault notional to swivel (from msg.sender)
    uint256 fee = principalFilled / feenominators[2];
    if (!mPlace.transferVaultNotionalFee(o.protocol, o.underlying, o.maturity, msg.sender, fee)) { revert Exception(10, 0, 0, address(0), address(0)); }

    emit Initiate(o.key, hash, o.maker, o.vault, o.exit, msg.sender, a, principalFilled);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Have o.maker withdraw his underlying instead of transferring it to them.

[JTraversa]

We intentionally dont plan to work with 777 tokens out of security concerns (same as most of our integrated protocols).

[bghughes]

https://github.com/code-423n4/2022-07-swivel/blob/main/Swivel/Swivel.sol#L125

I agree; this is a global comment with respect to the possibility for an ERC777 to spoof an ERC-20. I believe this issue is informational at best. You could consider adding nonReentrant modifier to the higher-order initiate to avoid unexpected callbacks.

[bghughes]

Grouping this with the warden’s QA report, #143