code423n4 commented 2 years ago

Handle

leastwood

Vulnerability details

Impact

The claim function in USDV is intended to be called when unlocking tokens previously locked after minting or burning. The claim function does not check if i is a valid array index, and as a result the call will revert with no relevant error message.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/main/contracts/tokens/USDV.sol#L122-L142

function claim(uint256 i) external onlyWhenNotLocked returns (uint256) {
Lock[] storage userLocks = locks[msg.sender];
Lock memory lock = userLocks[i];

require(lock.release <= block.timestamp, "USDV::claim: Vesting");

uint256 last = userLocks.length - 1;
if (i != last) {
    userLocks[i] = userLocks[last];
}

userLocks.pop();

if (lock.token == LockTypes.USDV)
    _transfer(address(this), msg.sender, lock.amount);
else vader.transfer(msg.sender, lock.amount);

emit LockClaimed(msg.sender, lock.token, lock.amount, lock.release);

return lock.amount;
}

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider adding a require statement with a relevant error message to ensure i < userLocks.length.

0xstormtrooper commented 2 years ago

As mentioned above, "call will revert with no relevant error message." Hence there is 0 risk.

jack-the-pug commented 2 years ago

Since the index can be reused, this can be misleading for users. I think it worth a Low.

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