Swivel v3 contest - 0xDjango's results

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L112-L114 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L132-L133

Vulnerability details

Impact

There is an error in the allowance functionality to allow a non-owner to withdraw or redeem ZcTokens for the owner. Taking ZcToken.redeem() as an example, behold the following if/else block:

        if (holder == msg.sender) {
            return redeemer.authRedeem(protocol, underlying, maturity, msg.sender, receiver, principalAmount);
        }
        else {
            uint256 allowed = allowance[holder][msg.sender];
            if (allowed >= principalAmount) { revert Approvals(allowed, principalAmount); }
            allowance[holder][msg.sender] -= principalAmount;  
            return redeemer.authRedeem(protocol, underlying, maturity, holder, receiver, principalAmount);

If the msg.sender is the holder, no check for allowance is needed. If the sender is not the holder, then their allowance is checked.

The error lies in the if statement if (allowed >= principalAmount) { revert Approvals(allowed, principalAmount); }. This states that if the sender has equal to or more allowance than the principalAmount, revert.

Therefore, if the sender has the proper allowance or more allowance than necessary, the transaction will revert. If the sender has less allowance than necessary, the transaction will still revert because of the allowance[holder][msg.sender] -= principalAmount; clause.

In conclusion, there is no way to withdraw() or redeem() on behalf of another user.

Tools Used

Manual review.

Recommended Mitigation Steps

The fix is to simply change >= to <

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/MarketPlace.sol#L156 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L620

Vulnerability details

Impact

A user redeems or withdraws from their ZcToken by calling ZcToken.withdraw() or ZcToken.redeem(). Both of these functions then call MarketPlace.authRedeem() which in turn calls Swivel.authRedeem(). The issue is that Swivel.sol does not have an authRedeem() function. Instead it only has the authRedeemZcToken() function.

Therefore, all redeem/withdraw operations will fail and all users' funds will be stuck.

Proof of Concept

1. User attempts to redeem their ZcToken that has matured with ZcToken.redeem()

• This function calls MarketPlace.authRedeem()

2. MarketPlace.authRedeem() calls ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, a);

• Since Swivel.sol does not contain this function, the call will revert and the user's ZcToken will not be redeemed. Their underlying will sit in the contract.

Tools Used

Manual review.

Recommended Mitigation Steps

Change the function name in Swivel.sol from authRedeemZcToken() to authRedeem().

QA Report

[L-01] Ownership transfer should be a two-step process

It is recommended that ownership transfer be completed in two steps, firstly by setting a pending admin and finally by accepting the change from the pending admin account.

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L428-L432 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/MarketPlace.sol#L53-L56

[L-02] Floating pragma

Certain contracts contain a floating pragma. It is encouraged to deploy with the same specific compiler version for all contracts.

E.g. https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L2 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/Erc20.sol#L4

[N-01] Comment vs Code

The comment refers to "owner" but the param is named "holder".

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L94 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L120

Gas Report

[G-01] ++i will save a bit of gas over i++

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L417-L419