imsrybr0 - Allo@allocate can lead to ETHbeing locked in strategies due to user error

[sherlock-admin2]

imsrybr0

medium

Allo@allocate can lead to ETHbeing locked in strategies due to user error

Pool managers / users (for strategies that allow it) can mistakenly send ETH to strategies while allocating recipients for a pool.

Vulnerability Detail

Allo@_allocate always sends the msg.value even if the pool is not created with the NATIVE token.

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Allo.sol#L352-L354

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Allo.sol#L492-L494

Impact

Locked ETH in strategies.

Code Snippet

function testAllocateWithMsgValue() public {
        vm.deal(vm.addr(10), 1 ether);

        uint256 poolId = _utilCreatePool(0);

        assertEq(address(strategy).balance, 0);
        assertEq(vm.addr(10).balance, 1 ether);
        assertFalse(address(token) == Native.NATIVE);

        vm.prank(vm.addr(10));
        allo().allocate{value: 0.5 ether}(poolId, bytes(""));

        assertEq(address(strategy).balance, 0);      
        assertEq(vm.addr(10).balance, 1 ether);  
    }

Tool used

Manual Review

Recommendation

Either revert when ETH is sent and the pool doesn't support it

    function _allocate(uint256 _poolId, bytes memory _data) internal {
        if (pools[_poolId].token != NATIVE && msg.value > 0) {
            revert("UNSUPPORTED_TOKEN");
        }

        pools[_poolId].strategy.allocate{value: msg.value}(_data, msg.sender);
    }

or adjust the logic to refund the sender in this scenario

    function _allocate(uint256 _poolId, bytes memory _data) internal {
        uint256 value;

        if (pools[_poolId].token == NATIVE) {
            value = msg.value;
        }

        pools[_poolId].strategy.allocate{value: value}(_data, msg.sender);

        if (msg.value > value) {
            (bool success,) = payable(msg.sender).call{value: msg.value}("");
            require(success, "Failed refund");
        }
    }