sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

tsvetanovv - The protocol will not work with tokens over 18 decimal #58

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

tsvetanovv

high

The protocol will not work with tokens over 18 decimal

Summary

In BondBaseOFDA.sol we have function _validateOracle()

Already at the beginning of the function we have a check if the tokens are in the given range:

223: if (payoutTokenDecimals < 6 || payoutTokenDecimals > 18) revert Auctioneer_InvalidParams();
224: if (quoteTokenDecimals < 6 || quoteTokenDecimals > 18) revert Auctioneer_InvalidParams();

The protocol itself uses any ERC20 tokens. Some tokens have more than 18 decimals (e.g. YAM-V2 has 24). If such a token is used that has more than 18 decimal places, the check in _validateOracle will not pass and the price for such a token cannot be validated.

Vulnerability Detail

See Summary

Impact

If the token with more than 18 decimals is used, the logic itself breaks and does not work.

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/bases/BondBaseOFDA.sol#L205-L224

function _validateOracle(
        uint256 id_,
        IBondOracle oracle_,
        ERC20 quoteToken_,
        ERC20 payoutToken_,
        uint48 fixedDiscount_
    )
        internal
        returns (
            uint256,
            uint256,
            uint256
        )
    {
        // Ensure token decimals are in bounds
        uint8 payoutTokenDecimals = payoutToken_.decimals();
        uint8 quoteTokenDecimals = quoteToken_.decimals();

        if (payoutTokenDecimals < 6 || payoutTokenDecimals > 18) revert Auctioneer_InvalidParams();
        if (quoteTokenDecimals < 6 || quoteTokenDecimals > 18) revert Auctioneer_InvalidParams();

Tool used

Manual Review

Recommendation

One solution is not to use tokens with more than 18 decimal.

Oighty commented 1 year ago

The system is not designed to use tokens with more than 18 decimals. The reasoning has to do with value scaling in the SDA pricing model. While this may not be explicitly stated, the decimal validation is pretty clear that this is intentional.

Additionally, there are very few tokens with more than 18 decimals (YAM-v2 is the only example I've seen and it is no longer an active project).

cvetanovv commented 1 year ago

Escalate for 10 USDC

The protocol uses any ERC20 tokens, according to the information you provided. Relative to what you have provided I have reported an issue. It is possible to use tokens with more than 18 decimals or to add some such token in the future and then the logic breaks.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

The protocol uses any ERC20 tokens, according to the information you provided. Relative to what you have provided I have reported an issue. It is possible to use tokens with more than 18 decimals or to add some such token in the future and then the logic breaks.

You've created a valid escalation for 10 USDC!

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.

hrishibhat commented 1 year ago

Escalation rejected

This is protocol design. From the docs:

In order to support a broad range of tokens, with different decimal configurations and prices, we calculate an optimal scaling factor for each market. Specifically, the scale adjustment allows the SDA to support tokens with 6 to 18 configured decimals and with prices that differ by up to 24 decimal places (if the tokens have the configured decimals, less if not). To implement this, the market creator must provide a scale adjustment factor and format the price correctly.

sherlock-admin commented 1 year ago

Escalation rejected

This is protocol design. From the docs:

In order to support a broad range of tokens, with different decimal configurations and prices, we calculate an optimal scaling factor for each market. Specifically, the scale adjustment allows the SDA to support tokens with 6 to 18 configured decimals and with prices that differ by up to 24 decimal places (if the tokens have the configured decimals, less if not). To implement this, the market creator must provide a scale adjustment factor and format the price correctly.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.