Swivel v3 contest - panprog's results

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L124 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L104-L106

Vulnerability details

Impact

When VaultTracker calculates yield for mature markets, it is calculated as $maturityRate/savedRate - 1$. This works for the first call, but it also saves current rate. If current rate is higher than maturityRate, then any subsequent call to any VaultTracker function with yield calculation will revert because yield will be negative. This includes redeemInterest function, effectively making the user lose all amount not yet redeemed.

Post maturity it's possible to call Swivel.combineTokens, MarketPlace.transferVaultNotional or execute an order calling Swivel.initiateVaultFillingVaultExit, exitVaultFillingVaultInitiate, exitVaultFillingZcTokenExit, all of which save new exchange rate.

It's also possible to intentionally lock Swivel's fee vault the same way by executing an order calling initiateVaultFillingZcTokenInitiate or initiateVaultFillingZcTokenInitiate, both of which call transferVaultNotionalFee, which locks Swivel's notional vault (redeem will now revert).

Possible loss of redeemable amount for the user scenario:

1. Bob initiates his position in nToken (by buying it from someone else).
2. He let it sit until maturity and just before maturity he sees an attractive price to sell nToken
3. Bob initiates selling his nToken by executing an order which calls exitVaultFillingVaultInitiate
4. However, due to network congestion his order executes in a few minutes after maturity, when the market is already set mature.
5. The order successfully executes, giving Bob some premium and decreasing his nToken to 0.
6. Bob now tries to redeem the accured interest from holding nToken and transaction reverts. Bob has lost all the redeemable tokens.

Proof of Concept

1. removeNotional: new exchange rate (exchangeRate > maturityRate) is saved into the vault

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L104-L106

2. redeemInterest: maturityRate < vlt.exchangeRate, yield is negative and reverts

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/VaultTracker/VaultTracker.sol#L123-L125

Recommended Mitigation Steps

if (maturityRate > 0) { // Calculate marginal interest
	if (maturityRate > vlt.exchangeRate) { // do not let yield to be negative
		yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
	}
} else {

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Tokens/ZcToken.sol#L99 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Tokens/ZcToken.sol#L92

Vulnerability details

Impact

If maturityRate is still 0 after maturity deadline (because no transactions setting maturityRate have been executed yet), then previewWithdraw calculated amount (used by ZcToken.withdraw function) is 0 and thus withdraw function will send 0 underlying tokens to user, which might be very confusing to user. Subsequent call to the same function will send him correct amount.

The same problem applies to all view functions in ZcToken contract - they use saved market maturityRate, which can be 0 even past deadline time and functions revert or return 0 in this case.

Incorrect withdrawal behaviour:

1. Bob has some ZcTokens.
2. Right at the time of maturity Bob tries to withdraw his underlying tokens by calling ZcToken.withdraw with some underlying amount.
3. Instead of receiving corresponding amount, Bob receives nothing (but transaction still succeeds and he uses gas for it).

Proof of Concept

1. withdraw: calculates previewAmount from previewWithdraw

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Tokens/ZcToken.sol#L99

2. previewWithdraw: multiplication by maturityRate returns 0

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Tokens/ZcToken.sol#L92

Recommended Mitigation Steps

Add getMaturityRate function to ZcToken, which will return either market's maturityRate or (if it's 0) current market's exchangeRate. Use this function instead of maturityRate everywhere across ZcToken.