safdie - Unchecked array bounds will cause an out-of-bounds error if `array_` is empty in `LibQuote.sol`

[sherlock-admin3]

safdie

Medium

Unchecked array bounds will cause an out-of-bounds error if array_ is empty in LibQuote.sol

Summary

Unchecked array bounds in LibQuote.sol will cause an out-of-bounds error if array_ is empty.

Root Cause

In the removeFromArray function, the line array_[index] = array_[array_.length - 1]; could cause an out-of-bounds error if array_ is empty. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibQuote.sol#L45-L50

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

1. The triggerOutOfBounds function attempts to access an index that is out of the array’s bounds, which can lead to unexpected behavior or crashes. In Solidity, this will revert the transaction.
2. If unchecked, such vulnerabilities can be exploited by attackers to manipulate the contract’s state or cause denial of service (DoS) by repeatedly triggering the out-of-bounds access.
3. Writing to an out-of-bounds index can overwrite other critical data in memory, leading to data corruption and unpredictable contract behavior.

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract ArrayBoundsPoC {
    uint256[] public array;

    constructor() {
        // Initialize the array with some values
        array.push(1);
        array.push(2);
        array.push(3);
    }

    function removeFromArray(uint256 item) public {
        uint256 index = getIndexOfItem(item);
        require(index != type(uint256).max, "Item not found");
        array[index] = array[array.length - 1];
        array.pop();
    }

    function getIndexOfItem(uint256 item) internal view returns (uint256) {
        for (uint256 i = 0; i < array.length; i++) {
            if (array[i] == item) {
                return i;
            }
        }
        return type(uint256).max;
    }

    function triggerOutOfBounds() public {
        // Intentionally cause an out-of-bounds write
        array[0] = array[array.length]; // This will cause an out-of-bounds error
    }
}

Mitigation

To prevent such issues, always ensure that array accesses are within bounds. Here’s an updated version of your removeFromArray function with bounds checking:

function removeFromArray(uint256[] storage array_, uint256 item) internal {
    require(array_.length > 0, "Array is empty");
    uint256 index = getIndexOfItem(array_, item);
    require(index != type(uint256).max, "Item not found");
    array_[index] = array_[array_.length - 1];
    array_.pop();
}