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

0 stars 0 forks source link

safdie - Incomplete validation of `filledAmount` will lead to bypassing position size limits in `PartyBPositionActionsFacet.sol` #14

Open sherlock-admin4 opened 1 week ago

sherlock-admin4 commented 1 week ago

safdie

Medium

Incomplete validation of filledAmount will lead to bypassing position size limits in PartyBPositionActionsFacet.sol

Summary

Incomplete validation of filledAmount in PartyBPositionActionsFacet.sol will lead to bypassing position size limits.

Root Cause

The openPosition and fillCloseRequest functions allow Party B to specify a filledAmount for a position or close request. However, the contract does not adequately validate this filledAmount, allowing Party B to provide excessively small or large values that could result in unwanted behavior. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/PartyBPositionActions/PartyBPositionActionsFacet.sol#L32 Same in PartyBPositionActionsFacetImpl.sol contract. There is also incomplete validation of filledAmount in openPosition and fillCloseRequest.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

A malicious Party B could exploit the lack of validation on filledAmount by providing excessively small or large amounts to manipulate the system.

Steps for exploit:

Small Position Size:

  1. Party B could call openPosition with a very small filledAmount (e.g., 1 wei).
  2. This would create a quote and position with a minuscule amount, leading to leftover quotes that fall below the minimum acceptable quote value. Example exploit code:
    // Calling openPosition with a very small filledAmount
    partyBFacet.openPosition(quoteId, 1, openedPrice, upnlSig);

    A malicious Party B could spam the system with multiple positions of tiny sizes, resulting in the contract storage becoming unnecessarily large, increasing gas costs for other users.

Large Position Size:

  1. Party B could provide an excessively large filledAmount that exceeds the intended limit for a single trade, bypassing any constraints on position size. Example exploit code:
    // Calling openPosition with a very large filledAmount
    partyBFacet.openPosition(quoteId, MAX_UINT256, openedPrice, upnlSig);

    Party B could bypass intended position size limits, opening overly large positions that exceed reasonable risk limits. This could lead to financial manipulation, causing unfair losses to other participants in the system.

Impact

  1. Tiny positions can cause contract bloat, unnecessary gas consumption, and cluttered state, without any real economic activity taking place.
  2. Large filledAmount values can bypass limits on acceptable position sizes, allowing malicious actors to open disproportionally large positions, leading to potential system instability or unfair financial gain.
  3. If a minuscule amount of the position is opened, the leftover open portion might remain indefinitely, consuming gas and clogging the system.

PoC

No response

Mitigation

Implement a check to ensure that filledAmount is within reasonable bounds, both for minimum and maximum size.

Example:

require(filledAmount >= MIN_POSITION_SIZE, "PartyBPositionActionsFacet: filledAmount too small");
require(filledAmount <= MAX_POSITION_SIZE, "PartyBPositionActionsFacet: filledAmount too large");