parseAllPrice not support the tokens whose decimal is greater than 18

Summary

parseAllPrice not support the token decimal is greater than 18, such as NEAR with 24 decimal. Since buyToken / sellToken is dependent on parseAllPrice, so users can't trade tokens larger than 18 decimal, but DODOv3 is intended to be compatible with all standard ERC20, which is not expected.

Vulnerability Detail

        // fix price decimal
        if (tokenDecimal != 18) {
            uint256 fixDecimal = 18 - tokenDecimal;
            bidDownPrice = bidDownPrice / (10 ** fixDecimal);
            bidUpPrice = bidUpPrice / (10 ** fixDecimal);
            askDownPrice = askDownPrice * (10 ** fixDecimal);
            askUpPrice = askUpPrice * (10 ** fixDecimal);
        }

If tokenDecimal > 18, 18 - tokenDecimal will revert

Impact

DODOv3 is not compatible the tokens whose decimal is greater than 18, users can't trade them.

Code Snippet

https://github.com/sherlock-audit/2023-06-dodo/blob/a8d30e611acc9762029f8756d6a5b81825faf348/new-dodo-v3/contracts/DODOV3MM/lib/MakerTypes.sol#L99-L106

Tool used

Manual Review

Recommendation

Fix decimal to 36 instead of 18

traceurl commented 1 year ago

fixed in https://github.com/DODOEX/new-dodo-v3/pull/32 now use parseRealAmount() in Types.sol to deal with token whose decimals is not 18

KuTuGu commented 1 year ago

Escalate There are two questions:

This issue does not seem to be the repeat of the other two issues, they refer to overflows where the sum of two oracle token decimals is greater than 36, while this issue targets overflows where a single token decimal is greater than 18
Sponsor indicates that it is mainly for chainlink tokens, and there are tokens greater than 18 decimals on the main network: NEAR: 24 decimals, address: https://etherscan.io/token/0x85f17cf997934a597031b2e18a9ab6ebd4b9f6a4, oracle: https://etherscan.io/address/0xC12A6d1D827e23318266Ef16Ba6F397F2F91dA9b

sherlock-admin2 commented 1 year ago

Escalate There are two questions:

This issue does not seem to be the repeat of the other two issues, they refer to overflows where the sum of two oracle token decimals is greater than 36, while this issue targets overflows where a single token decimal is greater than 18

Sponsor indicates that it is mainly for chainlink tokens, and there are tokens greater than 18 decimals on the main network: NEAR: 24 decimals, address: https://etherscan.io/token/0x85f17cf997934a597031b2e18a9ab6ebd4b9f6a4, oracle: https://etherscan.io/address/0xC12A6d1D827e23318266Ef16Ba6F397F2F91dA9b

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

maarcweiss commented 1 year ago

I agree with escalation. As > 18 decimals tokens are more than just one specific non standard erc20, and the likelihood is higher, I would agree with a medium

hrishibhat commented 1 year ago

Result: Medium Unique Considering this a valid medium

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status:

kutugu: accepted