Lines of code
<https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L98-L107&gt;
<https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L124-L137&gt;
<https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L155-L161&gt;
<https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L173-L190&gt;

Vulnerability details

Impact

The delegate() is used to delegate sender’s votes to someone else without the need to actually send the tokens. Here is a quick intro for the function:

delegate(address delegatee)
This function allows msg.sender to delegate their voting power to delegatee and the delegatee will vote on behalf of msg.sender. Note that the tokens are not transferred to delegatee, only the voting power. Delegation can be changed or revoked at any time by calling delegate again with a different delegatee or with address(0).

Caution on the last statement Delegation can be changed or revoked at any time by calling delegate again with a different delegatee or with address(0). If a voter doesn’t want to delegate his votes anymore to anyone he calls the function with address(0). But here in this contract, that actually is possible only for 1 voter, others will be permanently locked out of this functionality of revoking votes.

Proof of Concept

In ERC20MultiDelegate.sol there is the contract ERC20ProxyDelegator !!! This ERC20ProxyDelegator is only created if a voter wants to delegate his votes to anuninitialized to or target address, meaning only if there is no ERC20ProxyDelegator for that to or target address.

contract ERC20ProxyDelegator {
    constructor(ERC20Votes _token, address _delegate) {
        _token.approve(msg.sender, type(uint256).max);
        _token.delegate(_delegate);
    }
}

So the actual issue here is that whenever a voter initiates a delegateMulti, this calls _delegateMulti:

function _delegateMulti(
        uint256[] calldata sources,
        uint256[] calldata targets,
        uint256[] calldata amounts
    ) internal {
        
        //CODE...

        // Iterate until all source and target delegates have been processed.

        for (
            uint transferIndex = 0;
            transferIndex < Math.max(sourcesLength, targetsLength);
            transferIndex++
        ) {

            address source = transferIndex < sourcesLength
                ? address(uint160(sources[transferIndex]))
                : address(0);

            address target = transferIndex < targetsLength
                ? address(uint160(targets[transferIndex]))
                : address(0);

            uint256 amount = amounts[transferIndex];

            if (transferIndex < Math.min(sourcesLength, targetsLength)) {

                // Process the delegation transfer between the current source and target delegate pair.
                _processDelegation(source, target, amount);

            } else if (transferIndex < sourcesLength) {

                // Handle any remaining source amounts after the transfer process.
                _reimburse(source, amount);

            } else if (transferIndex < targetsLength) {

                // Handle any remaining target amounts after the transfer process.
                createProxyDelegatorAndTransfer(target, amount);

            }


            //CODE...
        }
        
    }

Now the voter wants to revoke his votes, so he either enters the statement which calls: createProxyDelegatorAndTransfer or _processDelegation.

It’s important to note here that whichever is called they both have a check to see whether the target address has a proxyDelegator:

   function createProxyDelegatorAndTransfer(
        address target,
        uint256 amount
    ) internal {
        
        address proxyAddress = deployProxyDelegatorIfNeeded(target);
        //CODE...
    }



function _processDelegation(
        address source,
        address target,
        uint256 amount
    ) internal {
          
        //CODE...

        deployProxyDelegatorIfNeeded(target);

        //CODE...
    }

So let’s get into deployProxyDelegatorIfNeeded:

function deployProxyDelegatorIfNeeded(
        address delegate
    ) internal returns (address) {
        address proxyAddress = retrieveProxyContractAddress(token, delegate);

        // check if the proxy contract has already been deployed
        uint bytecodeSize;
        assembly {
            bytecodeSize := extcodesize(proxyAddress)
        }

        // if the proxy contract has not been deployed, deploy it
        if (bytecodeSize == 0) {
            new ERC20ProxyDelegator{salt: 0}(token, delegate);
            emit ProxyDeployed(delegate, proxyAddress);
        }
        return proxyAddress;
    }

First it checks if there is a proxy already created for the delegate address which in our case will be address(0) because we want to revoke votes. Let’s get into retrieveProxyContractAddress:

function retrieveProxyContractAddress(
        ERC20Votes _token,
        address _delegate
    ) private view returns (address) {

        bytes memory bytecode = abi.encodePacked(
            type(ERC20ProxyDelegator).creationCode, 
             //@audit bytecode determined by the token AND the delegate
            abi.encode(_token, _delegate)
        );

        bytes32 hash = keccak256(
            abi.encodePacked(
                bytes1(0xff),
                address(this),
                uint256(0), // salt
                keccak256(bytecode)
            )
        );
        
        return address(uint160(uint256(hash)));
    }

Nothing fancy here, just returns the address hash of the proxy if there is any. Let’s assume in our case that no one has created a proxy with address(0) as a delegate so it doesn’t return a valid address and when that is checked back again in the deployProxyDelegatorIfNeeded:

uint bytecodeSize;
        assembly {
            bytecodeSize := extcodesize(proxyAddress)
        }

This will return 0 so we go into this if statement:

if (bytecodeSize == 0) {
            new ERC20ProxyDelegator{salt: 0}(token, delegate);
            emit ProxyDeployed(delegate, proxyAddress);
        }

This will create a new ERC20ProxyDelegator and will call the delegate in the constructor, keep in my that the delegate function is in constructor so it’s called only when new targets are introduced.

So far so good, we managed to revoke our votes. But now let’s try that one more time but this time keep in mind that there is an already created ERC20ProxyDelegator for address(0) as a delegate, target or to address, whatever you want to call it.

As you can see by the steps above now the second time a voter tries to revoke his tokens, he won’t be able to because no new ERC20ProxyDelegator will be created, meaning the delegate function in the constructor of ERC20ProxyDelegatorWON’T be called, thus now everyone else trying to revoke will be unable to.

Tools Used

Manual Audit

Resources:

1.) <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/governance/utils/Votes.sol&gt;

2.) <https://www.rareskills.io/post/erc20-votes-erc5805-and-erc6372&gt;

Recommended Mitigation Steps

Think of a way for a voter to be able to revoke his tokens.

Assessed type

Context

---

The text was updated successfully, but these errors were encountered:

All reactions