Funds will not transfeered to Validator if it’s a contract without fallback or receive

[hats-bug-reporter[bot]]

Github username: -- Submission hash (on-chain): 0xbd9d03097ba8802700da1cbe677718f13d8465caa294b375c3ac930723ab7a4f Severity: medium

Description: Description\ Loss of reward for ‘validator’, when it is a contract that doesn’t have fallback or receive function.

Impact\ The issue is that withdrawalAddress could be a contract that does not have a fallback function. In this context rewardAddress.call will fail and eth would be transferred back to the contract.

Attachments

https://github.com/dappnode/mev-sp-contracts/blob/3929e24ea288d697d38948b8690c8c2028e5042b/contracts/DappnodeSmoothingPool.sol#L327

1. Proof of Concept (PoC) File

claimRewards is used for claiming reward but will revert for a contract that does not have receive or fallback function.

    function claimRewards(
        address withdrawalAddress,
        uint256 accumulatedBalance,
        bytes32[] memory merkleProof
    ) external {

        // Verify the merkle proof
        bytes32 node = keccak256(
            abi.encodePacked(withdrawalAddress, accumulatedBalance)
        );

        require(
            MerkleProofUpgradeable.verify(merkleProof, rewardsRoot, node),
            "DappnodeSmoothingPool::claimRewards: Invalid merkle proof"
        );

        // Get claimable ether
        uint256 claimableBalance = accumulatedBalance -
            claimedBalance[withdrawalAddress];

        // Update claimed balance mapping
        claimedBalance[withdrawalAddress] = accumulatedBalance;

        // Load first the reward recipient for gas saving, to avoid load twice from storage
        address currentRewardRecipient = rewardRecipient[withdrawalAddress];
        address rewardAddress = currentRewardRecipient == address(0)
            ? withdrawalAddress
            : currentRewardRecipient;
        // Send ether
        (bool success, ) = rewardAddress.call{value: claimableBalance}(
            new bytes(0)
        );
        require(
            success,
            "DappnodeSmoothingPool::claimRewards: Eth transfer failed"
        );
        emit ClaimRewards(withdrawalAddress, rewardAddress, claimableBalance);
    }

2. Recommendation

Either enforce that proposer is an EOA or take in a recipient address for ETH transfers.

[invocamanman]

That's one of the reason that we let the withdrawal address to set a different rewardRecipient.