code-423n4 / 2024-07-karak-findings

0 stars 0 forks source link

The manager of a `NativeVault` can prevent a user to withdraw their funds during a certain time. #72

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L334-L336 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L282

Vulnerability details

Impact

An operator can pause the NativeNode::withdraw() function which always revert when finishWithdrawal() is called and thus prevent the user from withdrawing his ETH.

Proof of Concept

When an operator want to create a NativeVault, he call the deployVaults() function on the Core contract with a custom config struct to give informations for the deployment. The extraData params is used to give the manager, slahsStore and nodeImplementation addresses on the initialize() function of the NativeVault.

The operator set a manager role which can be useful for giving authority on some functions for a custom vault developed for specifics purposes. We can see on the NativeVault, the owner and also the manager can pause the NativeNode functions:

    function pauseNode(INativeNode node, uint256 map) external onlyRolesOrOwner(Constants.MANAGER_ROLE) {
        node.pause(map);
    }

If the manager pause the withdraw() function on the NativeNode, it create a DoS of the withdrawal process, because when the user try to finalize a withdrawal, the call will always revert.

We can also note than also when the withdraw() function is paused, it's impossible to make a snapshot of the NativeVault.

At the end, the protocol can unpause the withdraw() function and revoke the manager privilege on the NativeVault to let the user withdraw, this issue is considered as a temporary freezing of funds. Note that if the administrator/team needs to revoke the manager privilege, the functionality of the custom manager authority on a custom vault is broken.

Tools Used

Manual Review

Recommended Mitigation Steps

In my opinion you can delete the whenFunctionNotPaused modifier on the NativeNode::withdraw() function to avoid potentials malicious behavior of the manager role.

Assessed type

Context

c4-judge commented 2 months ago

MiloTruck marked the issue as satisfactory

c4-judge commented 2 months ago

MiloTruck removed the grade

c4-judge commented 2 months ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 2 months ago

MiloTruck marked the issue as unsatisfactory: Invalid

MiloTruck commented 2 months ago

This is intended. If the manager pauses withdraw(), withdrawals are meant to be temporarily paused so finishWithdrawal() should revert.