018.md

Sticky Bone Raven

Medium

Off-by-One Error in _getStakeholdersForMarket Leads to Potential Memory Out-of-Bounds Access

Summary

The function getStakeholdersForMarket in the MarketRegistry.sol contains an off-by-one error when populating the stakeholders array. Specifically, the loop variable i is directly used to index into the array, which causes memory misalignment since the array's indices start from 0. This bug can result in out-of-bounds memory writes, corrupted data, or runtime exceptions.

Root Cause

https://github.com/sherlock-audit/2024-11-teller-finance-update/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/MarketRegistry.sol#L1002

uint256 len = _set.length();

uint256 start = _page * _perPage;
if (start <= len) {
    uint256 end = start + _perPage;
    // Ensure we do not go out of bounds
    if (end > len) {
        end = len;
    }

    stakeholders_ = new address[](end - start);
    for (uint256 i = start; i < end; i++) {
        stakeholders_[i] = _set.at(i);
    }
}

The incorrect indexing logic in the loop:

stakeholders_[i] = _set.at(i); The loop iterates over the range [start, end) in set, but directly uses i as the index for stakeholders without adjusting for the array's base index (0).

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Incorrect memory access may lead to:

• Reverts: If accessing an index out of bounds in stakeholders_.
• Corrupted Data: Assigning incorrect values or overwriting unintended memory locations.
• Gas Wastage: Due to failed transactions and repeated calls.

PoC

No response

Mitigation

Adjust the indexing logic in the loop to account for the zero-based indexing of stakeholders_. Replace: stakeholders_[i] = set.at(i); With: stakeholders[i - start] = _set.at(i);

Corrected Code:

function _getStakeholdersForMarket(
    EnumerableSet.AddressSet storage _set,
    uint256 _page,
    uint256 _perPage
) internal view returns (address[] memory stakeholders_) {
    uint256 len = _set.length();

    uint256 start = _page * _perPage;
    if (start >= len) {
        return new address  }

    uint256 end = start + _perPage;
    if (end > len) {
        end = len;
    }

    stakeholders_ = new address[](end - start);
    for (uint256 i = start; i < end; i++) {
        stakeholders_[i - start] = _set.at(i); // Correct index adjustment
    }
}