sherlock-audit / 2023-12-dodo-judging

5 stars 4 forks source link

bareli - Gas Limitations: #24

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

bareli

medium

Gas Limitations:

Summary

Gas Limitations: The contract operations that loop over arrays (e.g., setTokensPrice) should be monitored for potential out-of-gas errors if the arrays are too large.

Vulnerability Detail

@> for (uint256 i = 0; i < tokens.length; ++i) { if (haveWrittenToken[i] == 1) continue;

        haveWrittenToken[i] = 1;
        address curToken = tokens[i];
        uint80 curTokenPriceSet = tokenPrices[i];
        //_checkUpAndDownPrice(curTokenPriceSet);

        {
            uint256 tokenIndex = state.priceListInfo.tokenIndexMap[curToken] - 1;
            curFlag = curFlag & ~(1 << tokenIndex);
        }

        // get slot price
        uint256 curTokenIndex = (state.priceListInfo.tokenIndexMap[curToken] - 1) / 2;
        uint256 slotIndex = curTokenIndex / MakerTypes.PRICE_QUANTITY_IN_ONE_SLOT;
        uint256 priceInfoSet = (state.priceListInfo.tokenIndexMap[curToken] - 1) % 2 == 1
            ? state.priceListInfo.tokenPriceNS[slotIndex]
            : state.priceListInfo.tokenPriceStable[slotIndex];

        priceInfoSet = stickPrice(
            priceInfoSet, curTokenIndex % MakerTypes.PRICE_QUANTITY_IN_ONE_SLOT, uint256(curTokenPriceSet)
        );

        // find one slot token
  @>      for (uint256 j = i + 1; j < tokens.length; ++j) {
            address tokenJ = tokens[j];
            uint256 tokenJOriIndex = (state.priceListInfo.tokenIndexMap[tokenJ] - 1);
            if (
                haveWrittenToken[j] == 1 // have written
                    || (state.priceListInfo.tokenIndexMap[curToken] - 1) % 2 != tokenJOriIndex % 2 // not the same stable type
                    || tokenJOriIndex / 2 / MakerTypes.PRICE_QUANTITY_IN_ONE_SLOT != slotIndex
            ) {
                // not one slot
                continue;
            }
            //_checkUpAndDownPrice(tokenPrices[j]);
            priceInfoSet = stickPrice(
                priceInfoSet, (tokenJOriIndex / 2) % MakerTypes.PRICE_QUANTITY_IN_ONE_SLOT, uint256(tokenPrices[j])
            );

            haveWrittenToken[j] = 1;
            {
                uint256 tokenIndex = state.priceListInfo.tokenIndexMap[tokenJ] - 1;
                curFlag = curFlag & ~(1 << tokenIndex);
            }
        }

        if ((state.priceListInfo.tokenIndexMap[curToken] - 1) % 2 == 1) {
            state.priceListInfo.tokenPriceNS[slotIndex] = priceInfoSet;
        } else {
            state.priceListInfo.tokenPriceStable[slotIndex] = priceInfoSet;
        }
    }
    state.heartBeat.lastHeartBeat = block.timestamp;
    ID3MM(_POOL_).setNewAllFlag(curFlag);

    emit SetPoolInfo(2);

Impact

out-of-gas errors if the arrays are too large.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo/blob/main/dodo-v3/contracts/DODOV3MM/D3Pool/D3Maker.sol#L227

Tool used

Manual Review

Recommendation

Set a limit on array size.

sherlock-admin2 commented 8 months ago

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

karanctf commented:

same as #35

nevillehuang commented 8 months ago

Invalid, setTokensPrice is an admin permissioned function, invalid based on sherlock OOG rules

  1. Out of Gas: Issues that result in Out of Gas errors either by the malicious user filling up the arrays or there is a practical call flow that results in OOG can be considered a valid medium or in cases of blocking all user funds forever maybe a valid high. Exception: In case the array length is controlled by the trusted admin/owner or the issue describes an impractical usage of parameters to reach OOG state then these submissions would be considered as low.