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

11 stars 6 forks source link

The ManagedWallet contract cannot transfer ETH which will result in ETH being locked in it #675

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 9 months ago

Lines of code

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

Vulnerability details

Details

    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
        }

The confirmation wallet confirms or rejects wallet proposals by sending a specific amount of ETH to the ManagedWallet contract. However, this contract only implements the logic of receiving ETH and does not implement the function of transferring ETH, which will result in all ETH sent to this contract being locked.

Impact

The ETH in this contract cannot be transferred, resulting in economic losses.

Proof of Concept

Here is a possible attack scenario:

  1. confirmationWallet transfers 1 eth to ManagedWallet contract (as msg.vaule only needs to satisfy >=.05 ether, this operation can also be successful)
  2. Due to the lack of an ETH transfer function in this contract, all ETH sent to this contract will be locked

Tools Used

Visual Studio Code, Manual Code Review

Recommended Mitigation Steps

Assessed type

ETH-Transfer

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #59

c4-judge commented 8 months ago

Picodes marked the issue as unsatisfactory: Out of scope