Skip to content

Incorrect Parameter Order

Solidity does not support named parameters, and of course, the order in which function parameters are passed is crucial. Mistakes in parameter ordering, mainly in functions with numerous parameters, are easily made. Such errors can have severe unintended consequences, especially when they affect accounting-related operations.

A perfect illustration of this issue is evident from this finding from a Sherlock contest on GMX. 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
function updateTotalBorrowing(
    PositionUtils.UpdatePositionParams memory params,
    uint256 nextPositionSizeInUsd,
    uint256 nextPositionBorrowingFactor
) internal {
    MarketUtils.updateTotalBorrowing(
        params.contracts.dataStore,
        params.market.marketToken,
        params.position.isLong(),
        params.position.borrowingFactor(),
        params.position.sizeInUsd(),
        nextPositionSizeInUsd,
        nextPositionBorrowingFactor
    );
}

Here, the order of the 4th and 5th parameters, params.position.borrowingFactor() and params.position.sizeInUsd(), is particularly noteworthy. In contrast, 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. Such a mistake can lead to erroneous accounting, causing imbalances in the system whenever the updateTotalBorrowing function is invoked. Ideally, such an error should have been detected during the testing phase, emphasizing the importance of thorough test suites for smart contracts.