code-423n4 / 2024-03-dittoeth-findings

0 stars 0 forks source link

Insufficient Validation of Bid Parameters as seen on `line 86-87` of the `BidOrdersFacet.sol`,Leading to Potential DoS Attack #11

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BidOrdersFacet.sol#L87 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/BidOrdersFacet.sol#L86

Vulnerability details

Impact

The root cause lies in the inadequate validation of bid parameters, specifically the calculation of bid value based solely on user-provided price and ERC amount. This oversight allows malicious actors to exploit the system by creating numerous bids with trivial sizes but nonsensically high prices, potentially causing a Denial-of-Service (DoS) attack on ask orders. The impact could lead to significant disruption of the protocol's functionality and compromise its integrity. When validating the minBidEth for a bid, is a product of the actual ercAmount that the user provided multiplied by the arbitrary price given by user. Therefore it is possible that a malicious actor to provide a nonsensically high price in order to pass a validation with a trivial ercAmount.

Proof of Concept

function _createBid(
        address sender,
        address asset,
        uint80 price,
        uint88 ercAmount,
        bool isMarketOrder,
        MTypes.OrderHint[] memory orderHintArray,
        uint16[] memory shortHintArray
    ) private returns (uint88 ethFilled, uint88 ercAmountLeft) {
 @>       uint256 eth = ercAmount.mul(price);
 @>      if (eth < LibAsset.minBidEth(asset)) revert Errors.OrderUnderMinimumSize();

In the above function, the malicious actor can create many bids this way with trivial size that result in any ask order with a reasonable size requiring more gas than is available within the Block gas limit to execute, therefore DoSing asks and shutting down the protocol.

Tools Used

Manual

Recommended Mitigation Steps

-Introduce a minErcAmount validation such that the users must always provide a nontrivial amount of tokens.

-Additionally, restrict the the valid price range that users may open bids/asks for around +- 80% of the market price or so.

Assessed type

DoS

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 5 months ago

(eth < LibAsset.minBidEth(asset)) is already in place to circumvent trivial bid sizes.

hansfriese commented 5 months ago

Intended behavior. When createBid() is invoked with a high price, it means that the user intends to purchase the asset at that elevated price. And the payment amount will not be negligible for the user.

c4-judge commented 5 months ago

hansfriese marked the issue as unsatisfactory: Invalid