sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

cammamoon - The 'Batch Allocate' function does not handle 'msg.value' properly. #954

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

cammamoon

medium

The 'Batch Allocate' function does not handle 'msg.value' properly.

Regarding the Batch Allocate function on Allo.sol contract, when utilizing the native token, it currently sends msg.value to each pool allocation without distributing it proportionally among the pools array sent by the user in the function. This can result in either depleting the funds of the Allo Contract or causing the transaction to revert.

Vulnerability Detail

This renders the batch allocate function unusable.

Impact

As Allo.sol is not intended to serve as a vault and should not hold tokens, I will classify this as having a medium impact.

Code Snippet

Anyone can invoke the batch allocate function, specifying multiple pools and including 'X' amount of a native token as 'value' in the transaction.

In the loop it will call _allocate, with the same 'msg.value' sent by the user

for (uint256 i; i < numPools;) {
            _allocate(_poolIds[i], _datas[i]);
            unchecked {
                ++i;
            }
        }

and then in the _allocate function it will send by each one of the pools the same msg.value:

  function _allocate(uint256 _poolId, bytes memory _data) internal {
        pools[_poolId].strategy.allocate{value: msg.value}(_data, msg.sender);
    }

Tool used

Manual Review

Recommendation

Prior to sending the amount to each pool, divide 'msg.value' by the number of pools.

sherlock-admin commented 11 months ago

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

n33k commented:

invalid, batchAllocate is not payable