Description

The mapping payerRails is defined as mapping(address => mapping(address => uint256[])) private payerRails;, which maps an address to another address<->array mapping. During creation of rails, the railId of the new rail is pushed onto this array.

payeeRails[token][to].push(railId);
payerRails[token][from].push(railId)

The array only keeps growing, and elements from it are never removed.

To fetch the information stored in this array, the contract provides these functions:

function getRailsForPayerAndToken(address payer, address token) external view returns (RailInfo[] memory) {
    return _getRailsForAddressAndToken(payer, token, true);
}

function _getRailsForAddressAndToken(address addr, address token, bool isPayer)
    internal
    view
    returns (RailInfo[] memory)
{
    // Get the appropriate list of rails based on whether we're looking for payer or payee
    uint256[] storage allRailIds = isPayer ? payerRails[token][addr] : payeeRails[token][addr];
    uint256 railsLength = allRailIds.length;

    RailInfo[] memory results = new RailInfo[](railsLength);
    uint256 resultCount = 0;

    for (uint256 i = 0; i < railsLength; i++) {
        uint256 railId = allRailIds[i];
        Rail storage rail = rails[railId];

        // Skip non-existent rails
        if (rail.from == address(0)) continue;

        // Add rail info to results
        results[resultCount] = RailInfo({railId: railId, isTerminated: rail.endEpoch > 0, endEpoch: rail.endEpoch});
        resultCount++;
    }
    // ...
}

Note that the function loops over the entire length of the array every time, skipping non-existent rails. Every round of the loop consumes some amount of gas, and at some point iterating over the array length will get a very high gas cost.

Impact

Currently the function is not called internally, and is meant to be invoked externally instead. Since this is an external view function, there are two types of calls, depending on if the call is part of a transaction or not. In a transaction, the total gas is quite limited and the entire transaction will revert when calling the function on an array that is too long. When using eth_call via the RPC, it is up to the RPC to set a gas limit, and this limit can be much higher to the point where this likely won't be a problem.

Recommendations

Add pagination or a start/stop index as parameters to the function.

Remediation

Filecoin Services has stated that they intend to address this issue at a later date.