veRWA - zhaojie's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L326 https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L356

Vulnerability details

Impact

After the user executes VotingEscrow.createLock, the VotingEscrow.delegate operation is performed. After the time-lock date expires, the delegate cannot be re-delegate to self. As a result, the user cannot execute withdraw and user funds are locked.

Proof of Concept

VotingEscrow.withdraw require locked_.delegatee being equal to self,if the user's LockedBalance is delegated,withdraw will fail.

    function withdraw() external nonReentrant {
        LockedBalance memory locked_ = locked[msg.sender];
        // Validate inputs
        require(locked_.amount > 0, "No lock");
        require(locked_.end <= block.timestamp, "Lock not expired");
        require(locked_.delegatee == msg.sender, "Lock delegated");
        .....
    }

Therefore, the user must first call VotingEscrow.delegate(msg.sender) when withdraw, to set locked_.delegatee to himself.

However, if the user does not execute VotingEscrow.delegate(msg.sender) before the time lock expires, the VotingEscrow.delegate execution will fail after the time lock expires. Because the delegate function depends on require toLocked.end > block.timestamp.

  function delegate(address _addr) external nonReentrant {
        LockedBalance memory locked_ = locked[msg.sender];
        ....
        require(toLocked.amount > 0, "Delegatee has no lock");
        //@audit cannot execute after the time lock expires
        require(toLocked.end > block.timestamp, "Delegatee lock expired");
        require(toLocked.end >= fromLocked.end, "Only delegate to longer lock");
        _delegate(delegatee, fromLocked, value, LockAction.UNDELEGATE);
        _delegate(_addr, toLocked, value, LockAction.DELEGATE);
    }

In this case, the user cannot execute the withdraw function, and user funds are locked.

test code:

contract VotingEscrowTest is Test {
    VotingEscrow public ve;

    address public constant userA = address(0x1000A);
    address public constant userB = address(0x1000B);

    uint256 public constant LOCK_AMT = 1 ether;

    function setUp() public {
        ve = new VotingEscrow("Voting Escrow", "VE");
        vm.deal(userA, 100 ether);
        vm.deal(userB, 100 ether);
    }

    function testWithdrawAfterTimeLockDelegated() public {
        vm.prank(userA);
        ve.createLock{value: LOCK_AMT}(LOCK_AMT);

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

        // userA delegate to userB
        vm.prank(userA);
        ve.delegate(userB);

        // userA withdraw after timelock
        vm.prank(userA);
        (, uint256 end, , ) = ve.locked(userA);
        vm.warp(end + 1);

        //re-delegate to self will fail
        vm.prank(userA);
        ve.delegate(userA); 

        vm.prank(userA);
        ve.withdraw();
    }
}

forge test --match-test WithdrawAfterTimeLockDelegated -vv

[⠒] Compiling... [⠑] Compiling 4 files with 0.8.17 [⠘] Solc 0.8.17 finished in 12.50s Compiler run successful!

Running 1 test for src/test/attack.t.sol:VotingEscrowTest [FAIL. Reason: Delegatee lock expired] testWithdrawAfterTimeLockDelegated() (gas: 864608) Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 2.16ms Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests: Encountered 1 failing test in src/test/attack.t.sol:VotingEscrowTest [FAIL. Reason: Delegatee lock expired] testWithdrawAfterTimeLockDelegated() (gas: 864608)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

vscode,foundry

Recommended Mitigation Steps

VotingEscrow.delegate allows delegatee to be set to itself after time has elapsed.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L268 https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/VotingEscrow.sol#L115

Vulnerability details

Impact

In VotingEscrow.createLock, if the amount of ether sent is less than LOCKTIME, the votingPower is 0(slope=0), which is inconsistent with the user's expected result.

Proof of Concept

Test code:

    //send 1 ether , Success
    function testSlope1() public {
        vm.prank(userA);
        uint256 x = 1 ether;
        console.log("x:%d", x);

        ve.createLock{value: x}(x);
        (int128 biasA, int128 slopeA, ) = ve.getLastUserPoint(userA);
        console.log("slopeA: %d,biasA: %d", uint128(slopeA), uint128(biasA));
        
        assertGt(slopeA, 0);
    }

    //Send a smaller amount of Ether , Fail
    function testSlope2() public {
        uint256 y = 1825 days - 1;
        console.log("y:%d", y);
        vm.prank(userD);
        ve.createLock{value: y}(y);
        (int128 biasD, int128 slopeD, ) = ve.getLastUserPoint(userD);
        console.log("slopeD: %d,biasD: %d", uint128(slopeD), uint128(biasD));

        assertGt(slopeD, 0);
    }

forge test --match-test Slope -vv

Running 2 tests for src/test/attack.t.sol:VotingEscrowTest
[PASS] testSlope1() (gas: 322651)
Logs:
  x:1000000000000000000
  slopeA: 6341958396,biasA: 997260267512249604

[FAIL. Reason: Assertion failed.] testSlope2() (gas: 274369)
Logs:
  y:157679999
  slopeD: 0,biasD: 0
  Error: a > b not satisfied [int]
    Value a: 0
    Value b: 0

Test result: FAILED. 1 passed; 1 failed; 0 skipped; finished in 1.33ms
Ran 1 test suites: 1 tests passed, 1 failed, 0 skipped (2 total tests)

Failing tests:
Encountered 1 failing test in src/test/attack.t.sol:VotingEscrowTest
[FAIL. Reason: Assertion failed.] testSlope2() (gas: 274369)

Encountered a total of 1 failing tests, 1 tests succeeded

Division overflow occurs when _checkpoint calculates slope. Therefore, the slope value is 0 when the value is smaller than LOCKTIME.

userOldPoint.slope = _oldLocked.delegated / int128(int256(LOCKTIME));
....
userNewPoint.slope = _newLocked.delegated / int128(int256(LOCKTIME));

Tools Used

vscode,foundry

Recommended Mitigation Steps

In VotingEscrow.createLock require(_value > LOCKTIME, "");

Assessed type

Error