Skip to content

SortedTroves. insert is vulnerable to developer error #1031

Description

@xavierlepretre

The function SortedTroves.insert takes in uint256 _NICR as a parameter. Further in the call stack, this value is compared to _troveManager.getNominalICR(_nextId) and _troveManager.getNominalICR(_prevId).

I understand it is the intention of the developer to have the insert function be called with a correct _NICR value.

However, I reckon the SortedTroves contract could protect itself form the risk that the developer does not follow the original intentions with this change:

-   _insert(troveManagerCached, _id, _NICR, _prevId, _nextId);
+   _insert(troveManagerCached, _id, troveManagerCached.getNominalICR(_id), _prevId, _nextId);

On the downside, it creates a couple extra SLOAD costs, mitigated by the fact that the storage locations are likely to have been written in the same transaction and so should cost WARM_STORAGE_READ_COST, plus, the rest of the function cold reads a lot of other storage locations.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions