sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

0 stars 0 forks source link

ghufranhassan - Potential Denial Of Service in ForceActionsFacet::settleAndForceClosePosition function #17

Open sherlock-admin3 opened 1 week ago

sherlock-admin3 commented 1 week ago

ghufranhassan

Medium

Potential Denial Of Service in ForceActionsFacet::settleAndForceClosePosition function

Summary

The settleAndForceClosePosition function in the ForceActionsFacet contract processes an array of updatedPrices. If this array is excessively large, it could lead to high gas consumption, potentially resulting in a Denial of Service (DoS) condition.

Vulnerability Detail

In the ForceActionsFacet contract, the settleAndForceClosePosition function processes an array called updatedPrices. This array is used to update prices for specific quotes during the settlement and forced closure of positions. The vulnerability arises from the potential for this array to be excessively large. An attacker could exploit this by submitting transactions with very large updatedPrices arrays, potentially leading to network congestion or preventing other users from executing their transactions. This could effectively create a DoS condition, where legitimate users are unable to interact with the contract.

Code Snippet

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacet.sol#L69L99

Recommendation

Please add array limit so that the contract will reject any input that exceeds this size, ensuring that no single transaction can use up an excessive amount of gas. This makes the contract more resilient to gas-related DoS attacks.

function settleAndForceClosePosition(
        uint256 quoteId,
        HighLowPriceSig memory highLowPriceSig,
        SettlementSig memory settleSig,
        uint256[] memory updatedPrices
    ) external notLiquidated(quoteId) whenNotPartyAActionsPaused {
    uint256 maxUpdatedPrices = 100; // Example limit, adjust based on gas cost analysis
       require(updatedPrices.length <= maxUpdatedPrices, "ForceActionsFacet: Too many updated prices"); //Check if exceeding limit
        QuoteStorage.Layout storage quoteLayout = QuoteStorage.layout();
        Quote storage quote = quoteLayout.quotes[quoteId];
        uint256 filledAmount = quote.quantityToClose;
        (uint256 closePrice, bool isPartyBLiquidated, int256 upnlPartyB, uint256 partyBAllocatedBalance) = ForceActionsFacetImpl.forceClosePosition(
            quoteId,
            highLowPriceSig,
            settleSig,
            updatedPrices
        );
        uint256[] memory newPartyBsAllocatedBalances = new uint256[](1);
        newPartyBsAllocatedBalances[0] = partyBAllocatedBalance;
        if (isPartyBLiquidated) {
            emit LiquidatePartyB(msg.sender, quote.partyB, quote.partyA, partyBAllocatedBalance, upnlPartyB);
        } else {
            emit SettleUpnl(
                settleSig.quotesSettlementsData,
                updatedPrices,
                msg.sender,
                AccountStorage.layout().allocatedBalances[msg.sender],
                newPartyBsAllocatedBalances
            );
            emit ForceClosePosition(quoteId, quote.partyA, quote.partyB, filledAmount, closePrice, quote.quoteStatus, quoteLayout.closeIds[quoteId]);
            emit ForceClosePosition(quoteId, quote.partyA, quote.partyB, filledAmount, closePrice, quote.quoteStatus); // For backward compatibility, will be removed in future
        }
    }