For at least the first 3 days after launch the users will lose 99.999% of their value locked to fees
Summary
The fee math is incorrect in all lend methods in Lender.sol and results in almost all of the value input from users going to fees.
Vulnerability Detail
Currently, in all lend methods of Lender.sol (which will be the main entrypoints for users) the fee amount is calculated by the formula fee = amountLent / feenominator and then calculation for the actual value used from the user is like uint256 lent = amountLent - amountLent / feenominator;
The starting value of feenominator in the constructor is feenominator = 1000;. This means that if an ERC20 token with 18 decimals (the usual) was used as the underlying asset to lend, then the math will be like:
amountLent = 1e18;
fee = amountLent / feenominator => fee = 1e15;
amountLent -= fee => amountLent = 1000
This means that the user will lose 99.999% of his value lent to fees. This will be the case until the feenominator value is updated to a bigger value, but with the current implementation this takes at least 3 days.
Impact
The impact is loss of almost all of the value users input in the protocol and they can't get it back in a permissionless manner. Since this is certain to happen for the users in the first 3 days after the launch of the protocol, I think High severity is appropriate.
pashov
high
For at least the first 3 days after launch the users will lose 99.999% of their value locked to fees
Summary
The fee math is incorrect in all
lend
methods inLender.sol
and results in almost all of the value input from users going to fees.Vulnerability Detail
Currently, in all
lend
methods ofLender.sol
(which will be the main entrypoints for users) the fee amount is calculated by the formulafee = amountLent / feenominator
and then calculation for the actual value used from the user is likeuint256 lent = amountLent - amountLent / feenominator;
The starting value of
feenominator
in the constructor isfeenominator = 1000;
. This means that if an ERC20 token with 18 decimals (the usual) was used as the underlying asset to lend, then the math will be like: amountLent = 1e18; fee = amountLent / feenominator => fee = 1e15; amountLent -= fee => amountLent = 1000This means that the user will lose 99.999% of his value lent to fees. This will be the case until the
feenominator
value is updated to a bigger value, but with the current implementation this takes at least 3 days.Impact
The impact is loss of almost all of the value users input in the protocol and they can't get it back in a permissionless manner. Since this is certain to happen for the users in the first 3 days after the launch of the protocol, I think High severity is appropriate.
Code Snippet
https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L483
Tool used
Manual Review
Recommendation
Use a different formula for fees calculation or just set the
feenominator
value to a much bigger value