sherlock-audit / 2023-02-blueberry-judging

12 stars 5 forks source link

eierina - Do not copy arrays to resize #362

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

eierina

false

Do not copy arrays to resize

Summary

Some functions copy arrays when the final size is unknown.

Vulnerability Detail

In the following function from the project, since the resulting array size is unknown, it first allocates the max size, then copies to the actual size.

    function getPositionIdsByOwner(address owner)
        external
        view
        returns (uint256[] memory ids)
    {
        uint256[] memory matchingIds = new uint256[](nextPositionId);
        uint256 index;
        for (uint256 i = 0; i < nextPositionId; i++) {
            if (positions[i].owner == owner) {
                matchingIds[index] = i;
                index++;
            }
        }

        ids = new uint256[](index); // @audit QA GAS OPTIMIZE by not re-copying but just changing array count via assembly
        for (uint256 i = 0; i < index; i++) {
            ids[i] = matchingIds[i];
        }
    }

To avoid copying, just resize the array instead of copying.

    function getPositionIdsByOwner(address owner)
        external
        view
        returns (uint256[] memory matchingIds)
    {
        matchingIds = new uint256[](nextPositionId);
        uint256 index;
        for (uint256 i = 0; i < nextPositionId; i++) {
            if (positions[i].owner == owner) {
                matchingIds[index] = i;
                index++;
            }
        }

       assembly {
           mstore(matchingIds, index);
       }
    }

Impact

Excess gas consumpton.

Code Snippet

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/BlueBerryBank.sol#L315-L333

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/BlueBerryBank.sol#L396-L427

Tool used

Manual Review

Recommendation

Change BlueBerryBank's getPositionIdsByOwner and getPositionDebts as follows:

    function getPositionIdsByOwner(address owner)
        external
        view
        returns (uint256[] memory matchingIds)
    {
        matchingIds = new uint256[](nextPositionId);
        uint256 index;
        for (uint256 i = 0; i < nextPositionId; i++) {
            if (positions[i].owner == owner) {
                matchingIds[index] = i;
                index++;
            }
        }

       assembly {
           mstore(matchingIds, index);
       }
    }