Collateral deposit function can break when fee is amount are too small

[code423n4]

Lines of code

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

Vulnerability details

Impact

When the deposit() function is called, the function confirms that, if a depositFee is set, the resulting fee is greater than zero.

if (depositFee > 0) { require(_fee > 0, "fee = 0"); }

However, deposits are likely to be made in USDC, which only has 6 decimals. Meanwhile, the depositFee is recorded as a value out of 1 million.

As a result, if depositFee is set to a very small value, then the function will revert for small USDC deposits.

Proof of Concept

Let's say the depositFee is set to 1:

• the fee is calculated as: uint256 _fee = (_amount * depositFee) / FEE_DENOMINATOR;
• the result will equal zero for any value where _amount * depositFee < FEE_DENOMINATOR.
• rearranging, it will equal zero when _amount < FEE_DENOMINATOR / depositFee
• solving, this means it will equal zero when _amount < 1 million
• with 6 decimals, this is equal to $10 USDC, which is a reasonable value to have not revert

Tools Used

Manual Review

Recommended Mitigation Steps

There's no need to require that fee > 0. Instead, simplify to only check that _amount > 0.

[Picodes]

Duplicate #275

[c4-judge]

Duplicate of https://github.com/code-423n4/2022-12-prepo-findings/issues/256