code-423n4 / 2021-12-vader-findings

0 stars 0 forks source link

USDV LockCreated event should include the index of a created lock #117

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

hyh

Vulnerability details

Impact

USDV claim is hardly usable when there are several claims in place as it requires index which user have to calculate themself.

Proof of Concept

User facing claim requires a user to supply claims array index, which wasn't communicated previously: https://github.com/code-423n4/2021-12-vader/blob/main/contracts/tokens/USDV.sol#L122

Recommended Mitigation Steps

The index is easy to communicate in a corresponding event, so a user can save it right after mint/burn for future usage. This is easy to be done programmatically and is useful for integration with other systems.

Now:

function _createLock(LockTypes lockType, uint256 amount) private {
        ...

        locks[msg.sender].push(Lock(lockType, amount, release));

        emit LockCreated(msg.sender, lockType, amount, release);
}

To be:

function _createLock(LockTypes lockType, uint256 amount) private {
        ...

        locks[msg.sender].push(Lock(lockType, amount, release));

        emit LockCreated(msg.sender, lockType, amount, release, locks[msg.sender].length - 1);
}

Also, LockClaimed event can communicate an index moved to the requested index position: if there was a copy, it's equal to the old last element index that was moved to i position, if there was no copy, it's zero.