[WP-M2] Wrong implementation of `TurboSafe.sol#less()` may cause boosted record value in TurboMaster bigger than actual lead to `BoostCapForVault` and `BoostCapForCollateral` to be permanently occupied

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L225-L236

Vulnerability details

https://github.com/code-423n4/2022-02-tribe-turbo/blob/66f27fe51083f49f7935e3fe594ab2380b75dee8/src/TurboSafe.sol#L225-L236

// Get out current amount of Fei debt in the Turbo Fuse Pool.
uint256 feiDebt = feiTurboCToken.borrowBalanceCurrent(address(this));

// If our debt balance decreased, repay the minimum.
// The surplus Fei will accrue as fees and can be sweeped.
if (feiAmount > feiDebt) feiAmount = feiDebt;

// Repay Fei debt in the Turbo Fuse Pool, unless we would repay nothing.
if (feiAmount != 0) require(feiTurboCToken.repayBorrow(feiAmount) == 0, "REPAY_FAILED");

// Call the Master to allow it to update its accounting.
master.onSafeLess(asset, vault, feiAmount);

In the current implementation, when calling less() to withdraw Fei from the Vault and use it to repay debt, if the amount of Fei is bigger than the debt balance, the onSafeLess hook will use feiDebt as The amount of Fei withdrawn from the Vault.

As a result, getTotalBoostedForVault[vault] in TurboMaster will be larger than the actual total amount of Fei being used to boost the Vault.

Since the Turbo Gibber may impound some of the Safe's collateral and mint a certain amount of Fei and repay the Safe's Fei debt with the newly minted Fei. In that case, the Safe's debt balance can be less than the amount of Fei in Vault. Which constitutes the precondition for the less() call to case the distortion of getTotalBoostedForVault[vault].

PoC

Given:

• 1 WBTC = 100,000
• collateralFactor of WBTC = 0.6
• getBoostCapForCollateral[WBTC] = 300,000
• getBoostCapForVault[vault0] = 300,000

1. Alice create Safe and deposit 10 WBTC and Boost 300,000 Fei to vault0

• Safe's debt = 300,000
• Safe's Fei in vault = 300,000

On master:

• getTotalBoostedForVault[vault0] = 300,000
• getTotalBoostedAgainstCollateral[WBTC] = 300,000

On safe:

• getTotalFeiBoostedForVault[vault0] = 300,000
• totalFeiBoosted = 300,000

2. WBTC price drop to 50,000, Turbo Gibber impound 2 WBTC and mint 100,000 Fei to repay debt for Alice's Safe.

• Safe's debt = 200,000
• Safe's Fei in vault0 = 300,000

3. Alice call less() withdraw 300,000 Fei from Vault and repay 200,000 debt, in the hook: master.onSafeLess(WBTC, vault0, 200,000)

• Safe's debt = 0
• Safe's Fei in vault = 0

On master:

• getTotalBoostedForVault[vault0] = 100,000
• getTotalBoostedAgainstCollateral[WBTC] = 100,000

On Safe:

• getTotalFeiBoostedForVault[vault0] = 0
• totalFeiBoosted = 0

4. Alice try deposit 20 WBTC and Boost 300,000 Fei will fail due to BOOSTER_REJECTED.

Recommendation

Change to:

function less(ERC4626 vault, uint256 feiAmount) external nonReentrant requiresLocalOrMasterAuth {
    // Update the total Fei deposited into the Vault proportionately.
    getTotalFeiBoostedForVault[vault] -= feiAmount;

    unchecked {
        // Decrease the boost total proportionately.
        // Cannot underflow because the total cannot be less than a single Vault.
        totalFeiBoosted -= feiAmount;
    }

    emit VaultLessened(msg.sender, vault, feiAmount);

    // Withdraw the specified amount of Fei from the Vault.
    vault.withdraw(feiAmount, address(this), address(this));

    // Call the Master to allow it to update its accounting.
    master.onSafeLess(asset, vault, feiAmount);

    // Get out current amount of Fei debt in the Turbo Fuse Pool.
    uint256 feiDebt = feiTurboCToken.borrowBalanceCurrent(address(this));

    // If our debt balance decreased, repay the minimum.
    // The surplus Fei will accrue as fees and can be sweeped.
    if (feiAmount > feiDebt) feiAmount = feiDebt;

    // Repay Fei debt in the Turbo Fuse Pool, unless we would repay nothing.
    if (feiAmount != 0) require(feiTurboCToken.repayBorrow(feiAmount) == 0, "REPAY_FAILED");
}

[transmissions11]

dupe of #30, but good find

[GalloDaSballo]

The warden identified a way to desynch the actual amounts of boostedFei for a vault and the system vs the amounts tracked in storage.

Because of this discrepancy the availability of borrowable FEI in the system can be distorted, preventing new borrows.

I do not believe this puts collateral at risk and also believe that the temporary "denial of borrowing" would be quickly fixed by raising caps.

I want to commend the warden for finding a way to break the system invariants, while the system internal accounting has been broken, no meaningful leak of value, extended denial of service or funneling of funds happened.

Liquidations can also still happen at the pool level.

Because of these reasons, I agree with Medium Severity