Interest surplus is accumulated on Master accounting update in TurboSafe.less

[code423n4]

Lines of code

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

Vulnerability details

Impact

There will be an interest surplus accumulating in all the master accounting variables (totalBoosted, getTotalBoostedForVault and getTotalBoostedAgainstCollateral).

As getTotalBoostedForVault and getTotalBoostedAgainstCollateral are used in the checks against getBoostCapForVault and getBoostCapForCollateral by booster.canSafeBoostVault function on the all new boosts, this will effectively reduce these caps over time (caps figures will remain the same, but vault figures will have a surplus accumulated, so the max possible difference will shrink over time).

This process will end up blocking all the boosts as vault/collateral values be filled up to their caps with surplus vault interest payments.

In other words, the sequence of boost -> slurp -> less (with amount = everything available) leaves safe's accounting variables unchanged as net effect will be zero, while master's variables mentioned above will be increased by the cumulative interest obtained from the vault.

As such kind of sequences are part of the usual workflow, the surplus interest component will fill the master's totals up to the point when they reach the caps and prohibit any new boosts, i.e. the contract become partly stuck, not allowing new funds in

Proof of Concept

Master.onSafeLess is called after the amount was reduced to the current debt in TurboSafe.less:

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

As safe accounts for full amounts in boost/slurp/less, the safe and master accounting will diverge, with safe's one being correct, and master's one bloated by the interest payments that were accounted in slurp before:

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol#L274-L289

Master totals are then used to validate new boosts:

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboMaster.sol#L232-L242

Recommended Mitigation Steps

Consider moving master.onSafeLess(asset, vault, feiAmount) up to the VaultLessened event emission, so the full feiAmount be accounted.

Now:

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

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

...

// 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);

To be:

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

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

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

...

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

[transmissions11]

looks like a dupe of #30?

[GalloDaSballo]

Duplicate of #55