Bypass Boosting cap set by Admin

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboRouter.sol#L118

Vulnerability details

Impact

Admin can set Boosting cap using setBoostCapForVault function at TurboBooster.sol#L100. This cap can be simply bypassed by delaying call to slurp function at TurboRouter.sol#L114 as shown in below POC

Proof of Concept

1. Admin has set Vault V boost cap as 100 at TurboBooster.sol#L59
2. User A calls the boost function with Vault V and feiAmount as 99 at TurboRouter.sol#L118
3. The boost will pass canSafeBoostVault function at TurboBooster.sol#L59 since getTotalFeiBoostedForVault[V] < boost cap (99<100)
4. Now there are 2 scenarios:

Scenario A (All goes good):

a. User A calls the slurp function at TurboRouter.sol#L114 and say interest earned is 3 with 1 as fees so getTotalFeiBoostedForVault[V] becomes 101

getTotalFeiBoostedForVault[V]=99+3-1=101

b. User B calls the boost function at TurboRouter.sol#L118 which reverts since canSafeBoostVault function at TurboBooster.sol#L59 fails. This happens since getTotalFeiBoostedForVault[V] becomes greater than boost cap (101>100)

Scenario B (The real problem)

a. Before anyone can make call to slurp function, User B calls the boost function with fei Amount as 1 at TurboRouter.sol#L118.

b. Boost is successful since canSafeBoostVault function at TurboBooster.sol#L59 passes.

c. This happens since getTotalFeiBoostedForVault[V] becomes 100 and boost cap is also 100 and since getTotalFeiBoostedForVault[V]=boost cap so canSafeBoostVault is success

d. As we can see in Scenario A User B was not able to deposit but if User B makes the transaction before slurp, he could boost

Recommended Mitigation Steps

Change the boost function like below:

function boost(TurboSafe safe, ERC4626 vault, uint256 feiAmount) public authenticate(address(safe)) {
safe.slurp(vault);        
safe.boost(vault, feiAmount);
    }

[Joeysantoro]

Unsure of what to do with this, I'm inclined to think yield is out of scope for the caps and that this is an ack but not fix

[transmissions11]

yeah hm cap dynamics are a bit weird with slurp, tho im also inclined to allow slurps to bypass caps but not other txs

[Joeysantoro]

closing as acknowledged, valid issue, but no action

[GalloDaSballo]

The warden identified a way to introduce more FEI on a Safe than the cap should allow. They were able to do so because the Yield for a Safe is not accounted for when checking limits, as such Safes over time will "reach the limit" and prevent further boosts in the system.

I believe over time the admins will have to increase caps, and ultimately at the time of renouncing of ownership they could just set the caps to uint256.max.

While I believe the finding is valid, it ultimately doesn't deny the protocol functionality except for very specific edge cases.

For these reasons I believe Low Severity to be more appropriate