veRWA - qpzm's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L331 https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L383

Vulnerability details

Impact

The delegator must extend his lock period by 5 years through increaseAmount and undelegate. After 5 years, he can finally withdraw. It is desirable that the delegator can withdraw when both locks expire.

Proof of Concept

Add the test in VotingEscrow.t.sol and run forge test --mt testDelegateExpired. I left comments to explain.

function testDelegateExpired() public {
    testSuccessCreateLock();
    (, uint256 end, , ) = ve.locked(user1);

    vm.prank(user2);
    ve.createLock{value: LOCK_AMT}(LOCK_AMT);

    // @audit user1 delegates to user2
    vm.prank(user1);
    ve.delegate(user2);

    int128 delegated;
    (, end, delegated, ) = ve.locked(user1);
    (, , delegated, ) = ve.locked(user2);
    assertEq(delegated, 2000000000000000000);

    // @audit delegatee user2 withdraws after his lock expires
    (, uint256 end2, , ) = ve.locked(user2);
    vm.warp(end2 + 1);
    vm.prank(user2);
    ve.withdraw();

    // @audit delegator user1 cannot undelegate and withdraw
    vm.prank(user1);
    vm.expectRevert("Delegatee lock expired");
    ve.delegate(user1);
    vm.prank(user1);
    ve.withdraw();
}

Tools Used

Foundry

Recommended Mitigation Steps

Skip toLocked.end > block.timestamp check when the msg.sender undelegates. This passes the test above.

function delegate(address _addr)
    external
    override
    nonReentrant
{
    // ...

    // @audit Skip toLocked.end > block.timestamp check when undelegating.
-    require(toLocked.end > block.timestamp, "Delegatee lock expired");
+    require(_addr == msg.sender || toLocked.end > block.timestamp, "Delegatee lock expired");
    
    // ...
}

https://github.com/code-423n4/2023-08-verwa/blob/main/src/VotingEscrow.sol#L383

Assessed type

Other