search

code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

Outdated Withdrawal Credential Setup #136

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L42

Vulnerability details

Impact

Withdrawals have changed from BLS public keys to Eth1 addresses. Unless the withdrawal credential is set to an Eth1 address, validators will NOT be able to withdraw from the beacon chain. They need to somehow coordinate a switch from BLS pubkeys to Eth1 addresses which will be a headache and actually gives validators full ability to withdraw funds to whatever addresses they want. This even violates trust assumptions if Frax extends their validator set to include esteemed staking service providers like StakeFish, Figment, or others.

Recommended Mitigation Steps

To solve this, Frax must setup a withdrawal stub contract which will receive withdrawn Ether from the beacon chain, then the OperatorRegistry must simply be changed to account for the newly formatted for the Eth1 withdrawal credential:

    bytes withdrawal_credential;
    address public timelock_address;

    constructor(address _owner, address _timelock_address, address _withdrawal_contract) Owned(_owner) {
        timelock_address = _timelock_address;
        withdrawal_credential= abi.encodePacked(byte(0x01), bytes11(0x0), _withdrawal_contract);
    }

Just need to make sure that all withdrawal contracts are maintained and kept ready for future consensus upgrades.

Super happy to see more liquid staking solutions popping up! Liquid staking is here to stay, and the more options ETH holders have, the better it is for the network :)

FortisFortuna commented 2 years ago

I don't think we will be using other validator services anytime soon, and if we do, we can always replace out this contract with updated code. Right now, we plan on just rolling our own.

0xean commented 1 year ago

Downgrading to QA