sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

0xarno - attacker can steal funds of allo.sol by using fundPool() function #971

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

0xarno

high

attacker can steal funds of allo.sol by using fundPool() function

TheAllo contract contains a vulnerability where an attacker can potentially steal Ether from the contract by sending a msg.value greater than _amount when using the fundPool` function.

Vulnerability Detail

In the Allo contract, there is a function called fundPool that allows anyone to deposit Ether into a pool. This function takes two arguments: _poolId and _amount. However, there is a vulnerability in this function that allows an attacker to send more Ether (msg.value) than the specified _amount. Lets suppose Allo hold 1 eth, malicious can just fund pool with amount = 100 eth, msg.value = 99 eth, which basically steal 1 eth from Allo contract

Impact

This vulnerability allows an attacker to steal Ether from the Allo contract by sending a larger msg.value than the intended _amount. This can result in a loss of funds for the contract and negatively impact the overall functionality of the Allo protocol.

Code Snippet

function _fundPool(uint256 _amount, uint256 _poolId, IStrategy _strategy) internal {
        uint256 feeAmount;
        uint256 amountAfterFee = _amount;

        Pool storage pool = pools[_poolId];
        address _token = pool.token;

        if (percentFee > 0) {
            feeAmount = (_amount * percentFee) / getFeeDenominator();
            amountAfterFee -= feeAmount;

            _transferAmountFrom(_token, TransferData({from: msg.sender, to: treasury, amount: feeAmount}));
        }

        _transferAmountFrom(_token, TransferData({from: msg.sender, to: address(_strategy), amount: amountAfterFee}));
        _strategy.increasePoolAmount(amountAfterFee);

        emit PoolFunded(_poolId, amountAfterFee, feeAmount);
    }

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/6430c8004017e96ae2f5aac365bdefd0b6eeea72/allo-v2/contracts/core/libraries/Transfer.sol#L70

Tool used

Manual Review

Recommendation

add a check that at the appropriate msg.value is sent

sherlock-admin commented 11 months ago

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

n33k commented:

invalid, Allo.sol is not meant to hold funds