sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

whiteh4t9527 - Allo.registerRecipient() is not necessary to be a payable function #961

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

whiteh4t9527

medium

Allo.registerRecipient() is not necessary to be a payable function

Since Allo.registerRecipient() does not pass the msg.value to the strategy, this function is not necessary to be a payable function. The received ETH via this function should only be refunded without triggering any effective logic.

Vulnerability Detail

Impact

The Allo contract would receive mistakenly sent ETH via registerRecipient().

Code Snippet

    function registerRecipient(uint256 _poolId, bytes memory _data) external payable nonReentrant returns (address) {
        // Return the recipientId (address) from the strategy
        return pools[_poolId].strategy.registerRecipient(_data, msg.sender);
    }

Tool used

Manual Review

Recommendation

Remove the payable keyword

sherlock-admin commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

n33k commented:

invalid, not a security issue