Protocol is not able to account for baseTokens generating yield

[code423n4]

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/3541bc704ab185a969f300e96e2f744a572a3640/apps/smart-contracts/core/contracts/Collateral.sol#L63-L78

Vulnerability details

Impact

The protocol's logic is based on the assumption that, while deposited, the underlying baseTokens will generate yield, which accrues to the Traders holding Collateral Tokens.

However, there is no mechanism in Collateral.sol to allow it to account for this yield.

Consulting with the sponsors, it seems that the short term plan is for a manager to withdraw funds from the contract and manually invest in tokens like yvUSDC. In the long term, this may be automated and included in the hook.

In either case, the result is that the protocol is able to turn its USDC into more USDC.

However, when a user goes to withdraw their balance, it is calculated without taking this increased USDC balance into account.

The result is that a Trader cannot benefit from the USDC yield, and it instead accrues to the Collateral.sol contract, and is accessible only by the manager via the managerWithdraw() function.

Proof of Concept

In the withdraw() function, we take in an amount of collateral tokens to burn.

This amount is converted to the amount to withdraw with the following formula:

uint256 _baseTokenAmount = (_amount * baseTokenDenominator) / 1e18;

This formula takes the decimals into account, but doesn't take into account the balance of the contract or the interest earned at all.

After this calculation is performed, the inputted amount of collateral tokens are burned, fees are taken, and the remainder is sent to the user. Interest is never accounted for.

Tools Used

Manual Review

Recommended Mitigation Steps

Use a standard like ERC4626 to account for increased vault balance and distribute that fairly to users.

[Picodes]

It's not clear to me that this is the intent of the code. At least from reading Collateral it seems the intent is that the yield is kept by the manager.

[c4-sponsor]

ramenforbreakfast marked the issue as sponsor disputed

[ramenforbreakfast]

Yes, as stated in the Collateral documentation, in the Collateral contract, only deposits are liabilities to Collateral holders, yield earned by deposits is kept by the manager.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid