code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Ether sent to `PendlePowerManager` will be lost forever #199

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/PowerFarms/PendlePowerFarm/PendlePowerManager.sol#L17-L29

Vulnerability details

Impact

In all other contracts in the protocol, we can see that the receive function looks like this:

    /**
     * @dev Standard receive functions forwarding
     * directly send ETH to the master address.
     */
    receive()
        external
        payable
    {
        if (msg.sender == WETH_ADDRESS) {
            return;
        }

        _sendValue(
            master,
            msg.value
        );
    }

This function gathers the ether sent to the contract without data and sends it to the master address. However, if we look at the PendlePowerManager, the comment says exactly the same but it only emits an event.

    /**
     * @dev Standard receive functions forwarding
     * directly send ETH to the master address.
     */
    receive()
        external
        payable
    {
        emit ETHReceived(
            msg.value,
            msg.sender
        );
    }

In this case the money is kept in the contract but there is no function to transfer it out. I guess that the contract needed the receive function open for the WETH contract interaction and some other things but it could be implemented just as other contract do. Check the sender to be one of the whitelisted contracts, if not, transfer the funds out to the master address or to the sender.

Tools Used

Manual review

Recommended Mitigation Steps

    /**
     * @dev Standard receive functions forwarding
     * directly send ETH to the master address.
     */
    receive()
        external
        payable
    {
        if (msg.sender == WETH_ADDRESS) {
            return;
        }

        _sendValue(
            master,
            msg.value
        );
    }

Assessed type

ETH-Transfer

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid

c4-judge commented 5 months ago

trust1995 marked the issue as satisfactory

c4-judge commented 5 months ago

trust1995 marked the issue as duplicate of #246

c4-judge commented 5 months ago

trust1995 marked the issue as selected for report

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid

trust1995 commented 5 months ago

Main discussion on #246