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

0 stars 0 forks source link

Delete locks before claim all finish execution #68

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

xnarus

Vulnerability details

Impact

For edge case claim all can fail and all user locks was deleted.

Proof of Concept

https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/USDV.sol#L150 https://github.com/code-423n4/2021-12-vader/blob/00ed84015d4116da2f9db0c68db6742c89e73f65/contracts/tokens/USDV.sol#L155

Tools Used

JetBrains

Recommended Mitigation Steps

0xstormtrooper commented 2 years ago

Transaction is atomic - either all claims are executed without failure or the entire transaction fails. There is no case where locks are deleted and user loses funds

If userLocks is empty then delete locks[msg.sender] has no effect.

Otherwise the for loop is executed. If the for loop fails, transaction will revert and undo delete locks[msg.sender]