Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Token with less then 6 decimals could incur precision loss

dontonka

Summary

The token amount (linked above) distributed to each supporter doesn't consider ERC20 decimals, which could be problematic in the future. According to the implementation BASIS_POINTS is at 10000, which means the computation could loose precision at some point when the numerator is less then 10000.

uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;

Vulnerability Details

For USDC and USDT, with a support of 6 decimals, seems like the minimum supported before starting loosing precision to be:

  • 1 cent prize only

  • 0.01% percentage for some supporter.

0.01% is passed as percentage (which would translate to 1) and if the totalAmount is 1 cent (which would translate into a value of 10000)
amount = 10000 * 1 / 10000 --> 1

So for those 2 tokens, anything that goes below those threshold would start loosing precision. Now Imagine if in the future a token is added which supports less than 6 decimals, that thresold goes up, and that become more problematic.

Impact

The impact of such precision loss will have a negative impact on the supporter in favor for the protocol as ALL the remaining token are transfered to the protocol at distribution time to ensure there is no dust in the contract, which is a good thing.

Tools Used

VS Code

Recommendations

I would deny any ERC20 token that supports less than 6 decimals. I would establish this in the ProxyFactory constructor as follow, which would allow:

  • Ensure the address are smart contract and seems ERC20 flavor as supporting decimals call

  • Ensure the ERC20 token has more than 6 decimals

diff --git a/src/ProxyFactory.sol b/src/ProxyFactory.sol
index c55c655..ada29db 100644
--- a/src/ProxyFactory.sol
+++ b/src/ProxyFactory.sol
@@ -78,14 +78,20 @@ contract ProxyFactory is Ownable, EIP712 {
* @notice the array is not supposed to be so long because only major tokens will get listed
* @param _whitelistedTokens The tokens array to get whitelisted
*/
constructor(address[] memory _whitelistedTokens) EIP712("ProxyFactory", "1") Ownable() {
if (_whitelistedTokens.length == 0) revert ProxyFactory__NoEmptyArray();
for (uint256 i; i < _whitelistedTokens.length;) {
if (_whitelistedTokens[i] == address(0)) revert ProxyFactory__NoZeroAddress();
+
+ if (ERC20(_whitelistedTokens[i]).decimals() < 6) revert ProxyFactory__TokenDecimalsNotSupported();
+
whitelistedTokens[_whitelistedTokens[i]] = true;
unchecked {
i++;
}

Furthermore, I would probably add some logic in Distributor::distribute to verify that the proxy hold enough funds in the token specified in parameter, and maybe even have a minimum hardcoded in the implementation, like maybe 10$ or something similar that make some sense in terms of minimal, and otherwise revert with a Distributor__NotEnoughFund.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.