search

sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

nmirchev8 - Stucked ETH in VotingMerkleDistributionStrategy contract #227

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

nmirchev8

medium

Stucked ETH in VotingMerkleDistributionStrategy contract

In DonationVotingMerkleDistributionDirectTransferStrategy there is a receive() function to accept native tokens, but there is no way to get those funds out of the contract.

Vulnerability Detail

DonationVotingMerkleDistributionDirectTransferStrategy is the parent contract and neither of its child has a functionallity to do something with the contract's balance. There is a receive function to accept ETH from any address and from any call, where signature does not match existing methods.

   /// @notice Contract should be able to receive ETH
    receive() external payable {}

But such functionallity is not needed and could only lead to stucked resources.

Impact

Manual Review

Recommendation

sherlock-admin commented 1 year ago

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

n33k commented:

low, but better to have withdraw

NicolaMirchev commented 1 year ago

Escalate Such issue is marked as 'medium' by many big names in industry. I think it is important such problems to reach the final report and end clients, because it could have real funds loss/lock impact, which is the defenition for medium issue Examples of such issues marked as "medium": https://solodit.xyz/issues/m-5-the-stream-contract-is-designed-to-receive-eth-but-not-implement-function-for-withdrawal-sherlock-nouns-nounsdao-git https://solodit.xyz/issues/m-03-contract-can-receive-eth-but-has-no-withdraw-function-for-it-pashov-none-moleculevesting-markdown https://solodit.xyz/issues/m-02-contract-hibernationden-can-receive-eth-but-it-cant-be-withdrawn-pashov-none-bearcave-markdown https://solodit.xyz/issues/trst-m-9-vault-does-not-have-a-way-to-withdraw-native-tokens-trust-security-none-mozaic-archimedes-markdown_

sherlock-admin2 commented 1 year ago

Escalate Such issue is marked as 'medium' by many big names in industry. I think it is important such problems to reach the final report and end clients, because it could have real funds loss/lock impact, which is the defenition for medium issue Examples of such issues marked as "medium": https://solodit.xyz/issues/m-5-the-stream-contract-is-designed-to-receive-eth-but-not-implement-function-for-withdrawal-sherlock-nouns-nounsdao-git https://solodit.xyz/issues/m-03-contract-can-receive-eth-but-has-no-withdraw-function-for-it-pashov-none-moleculevesting-markdown https://solodit.xyz/issues/m-02-contract-hibernationden-can-receive-eth-but-it-cant-be-withdrawn-pashov-none-bearcave-markdown https://solodit.xyz/issues/trst-m-9-vault-does-not-have-a-way-to-withdraw-native-tokens-trust-security-none-mozaic-archimedes-markdown_

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

neeksec commented 1 year ago

Duplicate of #719. See #719 for validity.

Evert0x commented 1 year ago

Planning to reject escalation (and not duplicate with 719) as the use case of this function is not taken into account in the issue description.

NicolaMirchev commented 1 year ago

Planning to reject escalation (and not duplicate with 719) as the use case of this function is not taken into account in the issue description.

But it is actual issue, how you don't consider it duplication? The root cause is the same and my recommendation would fix their issue. Yes, their explanation and example is better and that is way are bonuses for that.

Evert0x commented 1 year ago

Result: Invalid Unique

My original point stands.

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status: