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

1 stars 0 forks source link

TurboSafe - should override `maxWithdraw` and `maxRedeem` #67

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-tribe-turbo/blob/main/src/TurboSafe.sol https://github.com/Rari-Capital/solmate/blob/1205a9067ff957ef8b0b003ff9d77c20ef9f2e0b/src/mixins/ERC4626.sol#L167 https://github.com/Rari-Capital/solmate/blob/1205a9067ff957ef8b0b003ff9d77c20ef9f2e0b/src/mixins/ERC4626.sol#L171

Vulnerability details

Impact

Considering the EIP https://eips.ethereum.org/EIPS/eip-4626, as withdraw must revert if it is not possible to withdraw assets, it is important to have an accurate maxWithdraw function. However, here, maxWithdraw does not account for the current max withdrawal in the cToken contract. Liquidity might be unavailable because of borrows, or withdrawal may be capped because of the amount of FEI borrowed .

Same goes for maxRedeem.

Proof of Concept

The TurboSafe contains 100 assets from a single actor, and as borrowed 10 FEI for boosting. The actor cannot withdraw the 100 assets at once as it has a debt.

Recommended Mitigation Steps

One could use https://github.com/compound-finance/compound-protocol/blob/4a8648ec0364d24c4ecfc7d6cae254f55030d65f/contracts/Comptroller.sol#L675 to check the available liquidity, and the balance of the cToken to check current liquidity of the fuse pool.

GalloDaSballo commented 2 years ago

Because FEI is borrowed at 0 interest, there should be no issue in redeeming all FEI at all times, except in case of an exploit.

The idea behind the finding is legitimate, however, in lack of a POC the finding fails to convince me it has any substance