027.md

Petite Spruce Mammoth

Medium

The logic for determining the feeCollector will introduce unintended behavior if an invalid address is passed as the affiliate fee collector in LibPartyBPositionsActions.sol

Summary

The logic for determining the feeCollector in LibPartyBPositionsActions.sol will introduce unintended behavior if an invalid address is passed as the affiliate fee collector.

Root Cause

The contract contains logic for determining the fee collector based on the affiliate or a default address. If the logic does not correctly validate these addresses or if there's a flaw in the condition that checks whether to assign an affiliate fee collector or fall back to a default one, it can be exploited. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibPartyBPositionsActions.sol#L46-L48 A malicious actor could attempt to manipulate the parameters in such a way that they can direct fees to their own address or to an unintended address. Also, if there are any asynchronous calls or multiple transactions trying to collect fees at the same time, this could lead to a race condition where fees are incorrectly allocated or double-allocated to one address. For example, if a fee is calculated and added to an address's balance, but before the transaction is finalized, another transaction modifies that balance again, it could lead to inaccuracies in fee distribution. Moreover, if the fee collection logic lacks sufficient validation checks (e.g., checking whether the fee collector address is non-zero and valid), an attacker could exploit this to manipulate fee collections.

Internal pre-conditions

Assume a user (User A) is trading using the smart contract. They have an affiliate that should collect a fee.

The logic for determining the fee collector checks if the affiliateFeeCollector is valid; otherwise, it falls back to defaultFeeCollector.

External pre-conditions

No response

Attack Path

An attacker (Attacker B) observes that they can change the affiliate parameter or the state of the affiliateFeeCollector during the execution of a transaction, targeting the fee calculation and collection.

By sending a transaction that sets their address as the affiliate, they can redirect the fees meant for User A’s affiliate to their own address.

Impact

The attacker collects fees from trades that were intended for a legitimate affiliate, effectively stealing funds from the original trader. This could lead to loss of trust in the trading platform if users believe their fees are not being collected as intended.

PoC

Exploitable logic:

function openPosition(uint256 quoteId, uint256 filledAmount, uint256 openedPrice) internal returns (uint256 currentId) {
    // Initialization
    QuoteStorage.Layout storage quoteLayout = QuoteStorage.layout();
    AccountStorage.Layout storage accountLayout = AccountStorage.layout();
    GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();

    // Logic to determine fee collector
    address feeCollector = appLayout.affiliateFeeCollector[quote.affiliate] == address(0)
        ? appLayout.defaultFeeCollector
        : appLayout.affiliateFeeCollector[quote.affiliate];
    
    // Example of potentially manipulable logic
    require(feeCollector != address(0), "PartyBFacet: Fee collector is invalid");
    
    // Collecting fees
    if (quote.orderType == OrderType.LIMIT) {
        require(quote.quantity >= filledAmount && filledAmount > 0, "PartyBFacet: Invalid filledAmount");
        accountLayout.balances[feeCollector] += (filledAmount * quote.requestedOpenPrice * quote.tradingFee) / 1e36;
    } else {
        require(quote.quantity == filledAmount, "PartyBFacet: Invalid filledAmount");
        accountLayout.balances[feeCollector] += (filledAmount * quote.marketPrice * quote.tradingFee) / 1e36;
    }
}

Mitigation

Ensure that both affiliateFeeCollector and defaultFeeCollector are validated properly to ensure they are not the zero address and are intended recipients.