Skip to content

pubkey-uniqueness check is broken/does nothing #338

@0xSheller

Description

@0xSheller

poking around the megapool flow and noticed the "pubkey already in use" guard in addValidator can't actually reject anything. The key it derives includes the megapool address, so two different megapools submitting the same pubkey land on two different storage slots, both empty, and both pass the require.

contracts/contract/megapool/RocketMegapoolManager.sol (Lines 38-47):

function addValidator(address _megapoolAddress, uint32 _validatorId, bytes calldata _pubkey)
    override external
    onlyLatestContract("rocketMegapoolManager", address(this))
    onlyLatestContract("rocketNodeDeposit", msg.sender)
{
    uint256 index = getUint(setCountKey);
    setUint(setCountKey, index + 1);
    uint256 encoded = (uint256(uint160(_megapoolAddress)) << 96) | _validatorId;
    setUint(keccak256(abi.encodePacked("megapool.validator.set", index)), encoded);
    // Add pubkey => megapool mapping and ensure uniqueness
    bytes32 key = keccak256(abi.encodePacked("validator.megapool", _megapoolAddress, _pubkey));
    require(getAddress(key) == address(0x0), "Pubkey in use");
    setAddress(key, _megapoolAddress);
}

The comment says "ensure uniqueness", but _megapoolAddress is part of the key. Megapool A registering pubkey Xwrites to keccak("validator.megapool", A, X); megapool B registering the same X writes to keccak("validator.megapool", B, X). The two slots never collide so the require always passes for distinct megapools.

ATM no code currently consumes it, that makes me think it was meant to be canonical "is this pubkey claimed" lookup and never got wired up.

It's not really game breaking, but it does provide a cheap way to grief a competitor by donating depositor eth into their validator, and distorts protocol accounting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions