User can create small positions due to the current implementation of checks

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/ShortOrdersFacet.sol#L35-L90

Vulnerability details

Proof of Concept

Protocol in some instances place an enforcal on the minimum order sizes before the bids have been matched, for example take a look at https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/ShortOrdersFacet.sol#L35-L90

    function createLimitShort(
        address asset,
        uint80 price,
        uint88 ercAmount,
        MTypes.OrderHint[] memory orderHintArray,
        uint16[] memory shortHintArray,
        uint16 shortOrderCR
    ) external isNotFrozen(asset) onlyValidAsset(asset) nonReentrant {
        MTypes.CreateLimitShortParam memory p;
        STypes.Asset storage Asset = s.asset[asset];
        STypes.Order memory incomingShort;

        // @dev create "empty" SR. Fail early.
        incomingShort.shortRecordId = LibShortRecord.createShortRecord(asset, msg.sender, SR.Closed, 0, 0, 0, 0, 0);

        uint256 cr = LibOrders.convertCR(shortOrderCR);
        if ((shortOrderCR + C.BID_CR) < Asset.initialCR || cr >= C.CRATIO_MAX_INITIAL) {
            revert Errors.InvalidCR();
        }

        // @dev minShortErc needs to be modified (bigger) when cr < 1
        p.minShortErc = cr < 1 ether ? LibAsset.minShortErc(asset).mul(1 ether + cr.inv()) : LibAsset.minShortErc(asset);
        p.eth = price.mul(ercAmount);
        p.minAskEth = LibAsset.minAskEth(asset);
        if (ercAmount < p.minShortErc || p.eth < p.minAskEth) revert Errors.OrderUnderMinimumSize();

        // For a short, need enough collateral to cover minting ERC (calculated using initialCR)
        if (s.vaultUser[Asset.vault][msg.sender].ethEscrowed < p.eth.mul(cr)) revert Errors.InsufficientETHEscrowed();

        incomingShort.addr = msg.sender;
        incomingShort.price = price;
        incomingShort.ercAmount = ercAmount;
        incomingShort.id = Asset.orderIdCounter;
        incomingShort.orderType = O.LimitShort;
        incomingShort.creationTime = LibOrders.getOffsetTime();
        incomingShort.shortOrderCR = shortOrderCR; // 170 -> 1.70x

        p.startingId = s.bids[asset][C.HEAD].nextId;
        STypes.Order storage highestBid = s.bids[asset][p.startingId];
        // @dev if match and match price is gt .5% to saved oracle in either direction, update startingShortId
        if (highestBid.price >= incomingShort.price && highestBid.orderType == O.LimitBid) {
            LibOrders.updateOracleAndStartingShortViaThreshold(asset, LibOracle.getPrice(asset), incomingShort, shortHintArray);
        }

        p.oraclePrice = LibOracle.getSavedOrSpotOraclePrice(asset);
        if (LibSRUtil.checkRecoveryModeViolation(asset, cr, p.oraclePrice)) {
            revert Errors.BelowRecoveryModeCR();
        }

        // @dev reading spot oracle price
        if (incomingShort.price < p.oraclePrice) {
            LibOrders.addShort(asset, incomingShort, orderHintArray);
        } else {
            LibOrders.sellMatchAlgo(asset, incomingShort, orderHintArray, p.minAskEth);
        }
    }

This function is useed to create a limit short in the market, now while creating shorts, protocol implements checks on the minimum accepoted values for some variables, i.e if (ercAmount < p.minShortErc || p.eth < p.minAskEth) revert Errors.OrderUnderMinimumSize();, but the issue is that, protocol wrongly assumes that having this check before matching the bids is the same with having them after the bids have been matched.

This is because, matching doesn't guarantee, that there is enough amount that can be provided for the requested short. As a result in some cases the order would get finalised below the minimum size after matching.

Would be key to note that while this can happen accidentally, a very sophisticated attacker can control this behavour, as he/she can attempt providing the price of bid. So a bot would be placed that will check ask/short lists and provide such bid, that will fill almost, but not all of the requested amount.

Impact

Protocol invariant of a minimum order size is not correctly enforced since the order's size is checked before matching and not after, essentially creating the ability to have small positions, which liquidators may not be interested to liquidate, which eventually leads to bad debt.

Recommended Mitigation Steps

Consider reimplementing the order size check and have it be after the order's matched instead.

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #286

[raymondfam]

See #286.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Insufficient proof