QA Report

[code423n4]

Low

Wrong boolean logic

incorrect boolean logic in ERC20.sol permit function

if (recoveredAddress != address(0) && recoveredAddress != owner) {
    revert Invalid(msg.sender, owner);
}

• Allows an attacker to set allowance[address(0)][attacker] = Any value.
• Note that ecrecover() returns 0x0 if v is any other number than 27 or 28.
• This allows a valid recovered address of address(0)

Impact:

• If ERC20 tokens from this contract get sent to address(0) (maybe for burning/removing supply purposes), an attacker can use approve() and transferFrom() address(0) to their address and retrieve the tokens.
• The Approval(owner, spender, value) event will also be emit even though in reality, the spender does not have approval of owner.

Affected:

• https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/Erc20.sol#L165
• https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Creator/Erc20.sol#L165
• https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/Erc20.sol#L165

Recommendations:

• I believe the desired logic to be

  require(recoveredAddress != address(0) && recoveredAddress == owner)

  (from https://github.com/Uniswap/v2-core/blob/8b82b04a0b9e696c0e83f8b2f00e5d7be6888c79/contracts/UniswapV2ERC20.sol#L91)

• If so, it should be recoveredAddress == address(0) || recoveredAddress != owner (DeMorgan's Law)

QA

Interface and Contract mismatch

In MarketPlace.sol:

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

ISwivel in Interfaces.sol contains the interface:

interface ISwivel {
  function authRedeem(uint8 p, address u, address c, address t, uint256 a) external returns (bool);
}

But Swivel.sol does not have such a function. (Closest to it) Only authRedeemZcToken().

Affected 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/Marketplace/MarketPlace.sol#L164

[robrobbins]

demorgan's and interface issues addressed via other tickets