code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

ManagedWallet implements the receive function but does not implement the transfer logic, causing ETH to be locked. #904

Closed c4-bot-7 closed 7 months ago

c4-bot-7 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L59

Vulnerability details

Impact

Since ManagedWallet implements the receive function but does not implement transfer logic, the ETH in the contract will be locked and cannot be used normally.

Proof of Concept

ManagedWallet implements the receive function, but does not provide transfer logic.

    receive() external payable
        {
        require( msg.sender == confirmationWallet, "Invalid sender" );

        // Confirm if .05 or more ether is sent and otherwise reject.
        // Done this way in case custodial wallets are used as the confirmationWallet - which sometimes won't allow for smart contract calls.
        if ( msg.value >= .05 ether )
            activeTimelock = block.timestamp + TIMELOCK_DURATION; // establish the timelock
        else
            activeTimelock = type(uint256).max; // effectively never
        }

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/ManagedWallet.sol#L59

The ManagedWallet contract can only receive ETH and cannot perform transfer operations, which will result in the loss of user funds.

Tools Used

vscode

Recommended Mitigation Steps

Add a way to safely transfer ETH within the contract, such as selfdestruct.

Assessed type

Payable

c4-judge commented 7 months ago

Picodes marked the issue as duplicate of #59

c4-judge commented 6 months ago

Picodes marked the issue as unsatisfactory: Out of scope