code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

QA Report #192

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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)

robrobbins commented 2 years ago
  1. ok
  2. ok
  3. ?
  4. this is a common pattern. see ERC20
robrobbins commented 2 years ago

adding maybe to revisit 3. WRT the vaulttracker.transferNotionalFrom comment. likely addressed elsewhere or a non issue but..

robrobbins commented 2 years ago

this (3) might be a different wording for the issue (in another report) that led to the change that was made in the VaultTracker that now compares the maturityRate against the exchangeRate (when setting exchangeRate) and takes the lower of the two thus preventing it from being set to 0

robrobbins commented 2 years ago

so, i think this is just a non existent scenario. you cant transfer the exchange rate to another vault.