HeapOrdering Manual Review Findings

HOG-01M: Potentially Ambiguous Functionality

Type	Severity	Location
Standard Conformity		HeapOrdering.sol:L289-L295

Description:

The getTail function yields the tail of the overall account array rather than the tail of the sorted heap array which can cause mis-use by the library's users.

Example:

Copy
289/// @notice Returns the address at the tail of the `_heap`.
290/// @param _heap The heap to get the tail.
291/// @return The address of the tail.
292function getTail(HeapArray storage _heap) internal view returns (address) {
293    if (_heap.accounts.length > 0) return getAccount(_heap, _heap.accounts.length).id;
294    else return address(0);
295}

Recommendation:

We advise a separate, dedicated getHeapTail function to be introduced that yields the ordered tree portion's tail member to its caller, increasing the utility of the library.

Alleviation:

The Morpho team opted to instead update the surrounding comments of the function better illustrating its purpose as the heap implementation in question is not meant for general use. As a result, we consider this exhibit adequately dealt with.

HOG-02M: Inexistent Association of Former Value

Type	Severity	Location
Input Sanitization		HeapOrdering.sol:L30-L36

Description:

The _formerValue argument is meant to indicate the current value of the particular _id in the heap structure, however, this is not validated and is instead assumed to be trusted input.

Impact:

An improper _formerValue combined with an improper _newValue will cause an incorrect operation to be performed on the heap that does not match the actual data the _id entry previously held.

Example:

Copy
30function update(
31    HeapArray storage _heap,
32    address _id,
33    uint256 _formerValue,
34    uint256 _newValue,
35    uint256 _maxSortedUsers
36) internal {
37    uint256 size = _heap.size;
38    uint256 newSize = computeSize(size, _maxSortedUsers);
39    if (size != newSize) _heap.size = newSize;
40
41    if (_formerValue != _newValue) {
42        if (_newValue == 0) remove(_heap, _id, _formerValue);
43        else if (_formerValue == 0) insert(_heap, _id, _newValue, _maxSortedUsers);
44        else if (_formerValue < _newValue) increase(_heap, _id, _newValue, _maxSortedUsers);
45        else decrease(_heap, _id, _newValue);
46    }
47}

Recommendation:

While the contract is a library and thus input sanitization can be performed at the application level, from a security standpoint this reliance is ill-advised. Instead, we advise the _formerValue to be queried directly on the heap structure to guarantee all operations are performed safely regardless of end-user usage. If gas optimization is a concern here (as the data entry is already read by the caller), we advise a delta system to be used instead (i.e. specifying a decrease as -5) which would optimize the if conditionals and allow the entry to be read once at the heap level.

Alleviation:

The Morpho team evaluated this exhibit and deduced that the gas drawback of an additional SLOAD operation does not offset the sanitization benefit achieved as the library is meant for internal usage and Morpho expects its team to internally pass in only proper values. As a result, we consider this exhibit as acknowledged.