people can steal others voting power by creating mlicous proxy contracts

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L198

Vulnerability details

Impact

bad actor can control over the other peoples voting power in delegate proxy contracts.

lets start from delegateMulti when somebody calls it it leads to _delegateMulti and it calls _processDelegation(source, target, amount); in the process of loop.

if you look at this function

function _processDelegation(
        address source,
        address target,
        uint256 amount
    ) internal {
        uint256 balance = getBalanceForDelegate(source);

        assert(amount <= balance);

        deployProxyDelegatorIfNeeded(target);
        transferBetweenDelegators(source, target, amount);

        emit DelegationProcessed(source, target, amount);
    }

it gonna deploy to if needed as the deployProxyDelegatorIfNeeded works that way

now lets look at the function 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;
    }

it checks if the proxy address exists already. if yes it will skip if not it generates address BUT if we look at how it generates

in retrieveProxyContractAddress

 function retrieveProxyContractAddress(
        ERC20Votes _token,
        address _delegate
    ) private view returns (address) {
        bytes memory bytecode = abi.encodePacked(
            type(ERC20ProxyDelegator).creationCode, 
            abi.encode(_token, _delegate)
        );
        bytes32 hash = keccak256(
            abi.encodePacked(
                bytes1(0xff),
                address(this),
                uint256(0), // salt
                keccak256(bytecode)
            )
        );
        return address(uint160(uint256(hash)));
    }

You can see there is no increasing NOUNCE in the hashing process which is allowing bad actore create address with frontrunning user and creating exat same hash and deploying it as proxy delegate and having control over it and when the user comes to this step deployProxyDelegatorIfNeeded will skip because it already deployed and bad actore has control over it

Proof of Concept

    function retrieveProxyContractAddress(
        ERC20Votes _token,
        address _delegate
    ) private view returns (address) {
        bytes memory bytecode = abi.encodePacked(
            type(ERC20ProxyDelegator).creationCode, 
            abi.encode(_token, _delegate)
        );
        bytes32 hash = keccak256(
            abi.encodePacked(
                bytes1(0xff),
                address(this),
                uint256(0), // salt
                keccak256(bytecode)
            )
        );
        return address(uint160(uint256(hash)));
    }

https://github.com/code-423n4/2023-10-ens/blob/ed25379c06e42c8218eb1e80e141412496950685/contracts/ERC20MultiDelegate.sol#L198

Tools Used

vs code

Recommended Mitigation Steps

• Consider adding Increasing Nounce to hashing process to sure the hash created is not dup or created before

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as duplicate of #684

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid