TurboGibber do not check the market value of the collateral removed

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L89-L92

Vulnerability details

Impact

When the value of the safe's collateral is more than its debt, an authorized user can use TurboGibber.impound to obtain safe's collateral at the expense of safe's owner.

For example, suppose some time after the safe was opened the collateral market value rose 20x, while owner didn't performed any operations and the safe now is substantially overcollateralized. A malicious authorized user can obtain all the excess collateral simply by running impound(safe, 0, 0.95 * total_collateral, attacker_address).

I.e. while TurboGibber is meant to deal with undercollaterized safes, as no market value checks are performed it can be used to steal the collateral from any overcollaterized safes.

This is user's principal fund loss kind of scenario, but due to autorization restrictions the severity looks to be medium

Proof of Concept

TurboGibber.impound only repays requested feiAmount and tries to obtain the provided collateral amount, not checking whether it is undercollateralized safe or vice versa:

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboGibber.sol#L89-L92

Recommended Mitigation Steps

Consider checking the market value of the collateral and debt repayment, limiting the amount of the collateral to be obtained by the market value of the FEI minted. This way the usage with undercollaterized safes will be fully allowed, while the abilitiy to exploit the function to obtain the collateral for free will be removed.

The check can be similar to the one implemented in TurboSavior:

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/modules/TurboSavior.sol#L117-L132

[GalloDaSballo]

The market value of assets is tracked by the fusePool (cToken) in repaying debt you're repaying it with the fei value that the pool is attributing to the tokens