code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

QA Report #13

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Missing checks on new Boost Cap

  1. setBoostCapForVault function at TurboBooster.sol#L59 is missing checks to see if newBoostCap>getBoostCapForVault[vault]

  2. This is required since vault might already be at old boost cap.

  3. Setting lower boost cap would mean that boosting is already overflowed in this vault

  4. In case if it is required to lower the boost cap then slurpAndLess function at TurboRouter.sol#L130 must be called to withdraw excess cap amount

Recommendation:

function setBoostCapForVault(ERC4626 vault, uint256 newBoostCap) external requiresAuth {
require(newBoostCap>getBoostCapForVault[vault], "Invalid boost");
        // Update the boost cap for the Vault.
        getBoostCapForVault[vault] = newBoostCap;

        emit BoostCapUpdatedForVault(msg.sender, vault, newBoostCap);
    }

More funds extracted than required - Lose Interest

  1. Debt can be paid using less function at TurboSafe.sol
  2. But it was observed that User might call less function with higher feiAmount than required
  3. In case if feiDebt<feiAmount, the difference feiAmount-feiDebt is kept as vault balance
  4. This causes interest loss over these funds and even sweep can withdraw only after getTotalFeiBoostedForVault[vault]=0

Recommendation: Calculate the feiDebt first and only withdraw the min(feiAmount,feiDebt) so that only required amount is withdrawn

Missing zero address checks

  1. setBooster, setClerk, setDefaultSafeAuthority at TurboMaster.sol are not checking whether the supplied address!=0

Emit function called early

  1. The emit function at multiple functions have been called before function end say emit VaultLessened at TurboSafe.sol#L220. This is incorrect since operation has not completed yet and function might revert based on condition ahead. Also this cause gas wastage
GalloDaSballo commented 2 years ago

Agree with the first finding, not super sure about mitigation as it would make the system more bloated

Second finding definitely worth investigating

Zero checks -> Agree by convention

Emit functions are being emitted early as a way to avoid reEntrancy, so I'm ambivalent on this.

Overall the report was short and sweet, no particular formatting was needed and it looks good.

Wish the warden put links to the findings to make it easier to check

GalloDaSballo commented 2 years ago

In judging am also adding #9

6/10

GalloDaSballo commented 2 years ago

Bumping to 7 to make it the winner 7/10

GalloDaSballo commented 2 years ago

After re-review I confirm 7/10 the simple formatting avoids confusion, and the, Caps, Loose Interest and #9 make the report unique

GalloDaSballo commented 2 years ago

L-01 Missing checks on new Boost Cap

L-02 More funds extracted than required - Lose Interest

L-03 Missing zero address checks

N-01 Emit function called early

L-04 Bypass Boosting cap set by Admin