sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

Arz - The QV strategy does not accept ether #894

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 12 months ago

Arz

medium

The QV strategy does not accept ether

QVBaseStrategy.sol is missing a receive() function so the QV strategies wont be able to receive ether and wont work.

Vulnerability Detail

The quadratic voting strategies are supposed to receive ether however QVBaseStrategy.sol is missing a receive() function so strategies like QVSimpleStrategy wont be able to receive ether and wont work.

The user can unknowingly create a pool that accepts ether and uses the QVSimpleStrategy but because the contract doesnt accept ether all the calls will revert when other people will be donating. If the user wants to receive anything then he will have to create a new pool that uses a token.

Impact

The QV Strategy will not work with ether and the user who created the pool will lose his funds because he payed for the fee and will have to pay again if he wants to create a working pool otherwise he will fail to get funding.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/6430c8004017e96ae2f5aac365bdefd0b6eeea72/allo-v2/contracts/strategies/qv-base/QVBaseStrategy.sol#L30

To test this you can add this function to QVSimpleStrategy.t.sol and as you will see it will fail.

function testRevert_sendEther() public {
     vm.startPrank(pool_manager1());
     vm.deal(pool_manager1(), 1 ether);
     (bool success, ) = address(qvSimpleStrategy()).call{value: 1 wei}("");
     require(success, "Ether transfer failed");
}

Tool used

Manual Review + Foundry

Recommendation

Add a receive() function to QVBaseStrategy so it can receive ether.

Duplicate of https://github.com/sherlock-audit/2023-09-Gitcoin-judging/issues/256

sherlock-admin commented 11 months ago

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

n33k commented:

low, no fund lose, only incompatible with NATIVE token, should consider to fix