sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

John_Femi - Admin or Member Can Inflate Pool Amount #967

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

John_Femi

medium

Admin or Member Can Inflate Pool Amount

There is a possibility for a re-entrant call to recall an increase in pool amount multiple times

Vulnerability Detail

The details are as follows:

Impact

High Impact, which in best case scenario is loss of data and DOS, which could cause users or admin to switch pools. Worst case scenario loss of funds.

Code Snippet

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

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);
    }

function allocate(bytes memory _data, address _sender) external payable onlyAllo onlyInitialized {
        _beforeAllocate(_data, _sender);
        _allocate(_data, _sender);
        _afterAllocate(_data, _sender);
    }

Tool used

Manual Review

Recommendation

Move non-reentrant checks to internal functions instead and add checks in the increasePoolAmount

sherlock-admin commented 11 months ago

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

n33k commented:

invalid, not exactly showing how it's possbile, sugguest demonstrate with a PoC