Potential Array Length Mismatch in Batch Operations

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/Cred.sol#L810-L883

Vulnerability details

Impact

The _validateAndCalculateBatch function checks for length equality between credIds_ and amounts_ arrays, but fails to verify the length of the priceLimits_ array. This oversight could lead to unexpected behaviour, including:
Index out of bounds errors if priceLimits_ is shorter than the other arrays.
Unused price limits if priceLimits_ is longer, potentially bypassing intended price checks.
Inconsistent application of price limits across the batch operation. These issues could result in economic losses for users due to unintended price executions or transaction failures.

Proof of Concept

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/Cred.sol#L810-L883
A malicious user could exploit this by passing arrays of different lengths:

function _validateAndCalculateBatch(
    uint256[] calldata credIds_,
    uint256[] calldata amounts_,
    uint256[] calldata priceLimits_,
    bool isBuy
)
    internal
    view
    returns (
        uint256 totalAmount,
        uint256[] memory prices,
        uint256[] memory protocolFees,
        uint256[] memory creatorFees
    )
{
    uint256 length = credIds_.length;
    if (length != amounts_.length) {
        revert InvalidArrayLength();
    }
    // Missing check: if (length != priceLimits_.length) revert InvalidArrayLength();

    // ... snip ...
}

This call would pass the initial length check but could lead to an out-of-bounds error or skipped price limit check for the third item.

uint256[] credIds = [1, 2, 3];
uint256[] amounts = [100, 200, 300];
uint256[] priceLimits = [1000, 2000]; // Missing the third price limit

// Call to batchBuyShareCred or batchSellShareCred
contract.batchBuyShareCred(credIds, amounts, priceLimits, curator);

Tools Used

manual review

Recommended Mitigation Steps

Add a check to ensure all input arrays have the same length:

function _validateAndCalculateBatch(
    uint256[] calldata credIds_,
    uint256[] calldata amounts_,
    uint256[] calldata priceLimits_,
    bool isBuy
)
    internal
    view
    returns (
        uint256 totalAmount,
        uint256[] memory prices,
        uint256[] memory protocolFees,
        uint256[] memory creatorFees
    )
{
    uint256 length = credIds_.length;
    if (length != amounts_.length || length != priceLimits_.length) {
        revert InvalidArrayLength();
    }

    // ...snip...
}

Implementing checks could mitigate the possibility of out-of-bounds access, the risk of runtime errors or incorrect logic execution due to mismatched array lenghts.

Assessed type

Invalid Validation