sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

fugazzi - UbiquityPool implementation doesn't support tokens with more than 18 decimals #204

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 9 months ago

fugazzi

medium

UbiquityPool implementation doesn't support tokens with more than 18 decimals

Summary

The implementation of UbiquityPool will revert when a collateral token with more than 18 decimals is configured in the system.

Vulnerability Detail

When a new collateral is added to the pool, the contract will store the difference of 18 and the collateral token decimals in the missingDecimals array.

poolStorage.missingDecimals.push(
    uint256(18).sub(ERC20(collateralAddress).decimals())
);

However, this will cause an underflow if the collateral token decimals are greater than 18 due to the checked unsigned integer math. Any decimal value greater than 18 will cause a revert in the addCollateralToken() function.

Impact

The protocol cannot support collateral tokens with decimals greater than 18.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L647-L650

Tool used

Manual Review

Recommendation

Instead of storing the difference of 18 - decimals in missingDecimals, store just the decimals and then normalize based on which value is greater. If the token's decimals are greater than 18, then normalize by dividing by decimals - 18, else multiply by 18 - decimals.

sherlock-admin2 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Literally, no tokens with 18 decimals and no need

sherlock-admin2 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Literally, no tokens with 18 decimals and no need

nevillehuang commented 8 months ago

Invalid, based on contest details, the only supported collateral are LUSD and DAI both of which has 18 decimals on mainnet, so this is invalid