Swivel v3 contest - rokinot's results

Lines of code

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

Vulnerability details

Impact

The description of the function says "Allows users to redeem zcTokens and withdraw underlying, boiling up from the zcToken instead of starting on Swivel". In order for the function to be called, it needs to pass the modifier authorized(marketPlace), where marketPlace is the address of a deployed market place contract, but the MarketPlace.sol smart contract doesn't call this function anywhere, essentially turning it impossible to be reached by anyone, including the administrator, and unabling the tokens withdrawl.

Proof of Concept

#L620

  function authRedeemZcToken(uint8 p, address u, address c, address t, uint256 a) external authorized(marketPlace) returns(bool) {

Tools Used

Recommended Mitigation Steps

Add a function to MarketPlace.sol which calls this function, or remove the modifier.

Non-critical

matureVault() returns a boolean value, but it's never checked anywhere in the code.

#L143

setFee() reverts if the input array has size larger than 4.

#L504

To fix this, check if the input array has size lower than 4.

Low risk

Operation reverts due to underflow until the admin sets maturityRate to a higher value

transferNotionalFrom() can transfer the exchangeRate to a different vault with lower maturityRate, which causes the yield calculation to revert until the admin sets maturityRate to a higher value. VaultTracker.sol

Multiple custom errors are impossible to reach

Some operations are using a boolean return value in order to revert using a custom error, but the functions they call either returns true (which passes the check) or reverts (which bypasses the if function), never calling the custom error.

#L229-L233 #L173-L176 #L134-L141 #L603-L606)

x = x + y is a cheaper operation than x += y

There are multiple instances in the code VaultTracker.sol

vlt.redeemable += interest;
vlt.notional += a;

vlt.redeemable += interest;

from.redeemable += interest;

to.redeemable += newVaultInterest;
to.notional += a;

sVault.redeemable += interest;

sVault.notional += a;

Swivel.sol

filled[hash] += a;

for loops length comparison can use != len instead of < len for cheaper gas

#L85 #L247 #L409 #L552

use constant/literals (e.g. type(uint256).max) instead of 2**x

#L549

++i will consume less gas than i++ if the optimizer isn't enabled

#L100 #L269 #L418 #L564