sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

vangrim - [MEDIUM] QVBaseStrategy.sol #941

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

vangrim

medium

[MEDIUM] QVBaseStrategy.sol

The QVBaseStrategy.sol smart contract lacks a receive() or fallback() function, making it unable to accept incoming native tokens (e.g., ETH on Ethereum, MATIC on Polygon).

Vulnerability Detail

Pools on the Allo ecosystem should be able to receive ERC20 tokens as well as native tokens, which will later be used in order to allocate and distribute in accordance with the attached strategy.

At the present moment, the QVBaseStrategy.sol lacks a receive() or fallback() which means that it will revert when the _transferAmountFrom is called from ****Allo.sol****.

Impact

Users utilizing the QVBaseStrategy.sol the QVSimpleStrategy.sol , or any strategy inheriting those mentioned above cannot use native tokens in their implementation, which seems to be an unintended bug.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

To allow the contract to accept native tokens on any EVM-compatible network, add a receive() function if the sole intention is to handle straightforward native token transfers. The function should be external and payable:

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