Skip to content

Incorrect Parameter Order

Solidity does not support named parameters, so the order in which arguments are passed determines their meaning. Mistakes in that order are easily made in functions with numerous parameters, and they carry severe unintended consequences where they affect accounting-related operations.

A finding from a Sherlock contest on GMX illustrates the problem. In the contract PositionUtils.sol, a function named updateTotalBorrowing is defined:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
function updateTotalBorrowing(
    PositionUtils.UpdatePositionParams memory params,
    uint256 nextPositionSizeInUsd,
    uint256 nextPositionBorrowingFactor
) internal {
    MarketUtils.updateTotalBorrowing(
        params.contracts.dataStore,
        params.market.marketToken,
        params.position.isLong(),
        // @audit borrowing factor and size are passed in the wrong order
        params.position.borrowingFactor(),
        params.position.sizeInUsd(),
        nextPositionSizeInUsd,
        nextPositionBorrowingFactor
    );
}

Within the MarketUtils library, the corresponding function has a slightly different signature:

1
2
3
4
5
6
7
8
9
function updateTotalBorrowing(
    DataStore dataStore,
    address market,
    bool isLong,
    uint256 prevPositionSizeInUsd,
    uint256 prevPositionBorrowingFactor,
    uint256 nextPositionSizeInUsd,
    uint256 nextPositionBorrowingFactor
) external {

The parameters prevPositionSizeInUsd and prevPositionBorrowingFactor are swapped compared to their order in the call from PositionUtils.sol. The mistake produces erroneous accounting and imbalances in the system on every invocation of updateTotalBorrowing. An error of this kind belongs to the test suite, which is where a mismatch between a call site and a signature surfaces before deployment.