sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

carrotsmuggler - ETH transfers susceptible to gas griefing / DOS. #869

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

carrotsmuggler

medium

ETH transfers susceptible to gas griefing / DOS.

ETH transfers susceptible to gas griefing / DOS.

Vulnerability Detail

The contract uses the safeTransferEth function from the SafeTransferLib library to transfer ETH. As stated in the SafeTransferLib library itself, this function is susceptible to gas griefing / DOS.

/// @dev Sends `amount` (in wei) ETH to `to`.
/// Reverts upon failure.
///
/// Note: This implementation does NOT protect against gas griefing.
/// Please use `forceSafeTransferETH` for gas griefing protection.
function safeTransferETH(address to, uint256 amount) internal

So if a pool operates with NATIVE tokens, and a recipient is set to receive tokens from this pool, they can run a very large pool in the receiving address contract's receive function to use up all the gas. This will lead to situations where payouts can be delayed or DOSd since admins/managers wont be able to send out funds to users in a for loop, like the ones used in the _distribute functions in multiple strategies.

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/strategies/donation-voting-merkle-base/DonationVotingMerkleDistributionBaseStrategy.sol#L624-L629

Impact

DOS due to gas-griefing

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/libraries/Transfer.sol#L51 https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/libraries/Transfer.sol#L76 https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/libraries/Transfer.sol#L89

Tool used

Manual Review

Recommendation

Use the forceSafeTransferETH function on Solady's SafeTransfer library to protect against gas griefing. This function also takes in a gasStipend which limits gas griefing attacks.