sherlock-audit / 2023-09-Gitcoin-judging

11 stars 7 forks source link

alymurtazamemon - The `safeTransferETH` function does not protect from the `gas griefing` attack #953

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

alymurtazamemon

medium

The safeTransferETH function does not protect from the gas griefing attack

The Solady's safeTransferETH does to save from storage writes or gas griefing attacks while transferring ethers to other addresses. The SafeTransferLib itself recommend the forceSafeTransferETH function for transferring ethers.

Vulnerability Detail

/// @notice Safe ETH and ERC20 transfer library that gracefully handles missing return values.
/// @author Solady (https://github.com/vectorized/solady/blob/main/src/utils/SafeTransferLib.sol)
/// @author Modified from Solmate (https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol)
///
/// @dev Note:
/// - For ETH transfers, please use `forceSafeTransferETH` for gas griefing protection.
/// - For ERC20s, this implementation won't check that a token has code,
/// responsibility is delegated to the caller.
library SafeTransferLib {

As mentioned in the SafeTransferLib library of Solady => For ETH transfers, please use forceSafeTransferETH for gas griefing protection.

/// @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 {

Also, as mentioned above the safeTransferETH function of SafeTransferLib, this function does not protect against gas griefing attacks, you should use the forceSafeTransferETH to safely transfer the ethers.

Impact

All these below-mentioned spots are vulnerable to storage writes or gas griefing attacks.

Code Snippet

Transfer.sol - Line 51

Transfer.sol - Line 76

Transfer.sol - Line 89

DonationVotingMerkleDistributionDirectTransferStrategy.sol - Line 57

DonationVotingMerkleDistributionVaultStrategy.sol - Line 119

Tool used

Manual Review

Recommendation

-   SafeTransferLib.safeTransferETH(address(this), amount);
+   SafeTransferLib.forceSafeTransferETH(address(this), amount, gasStipend);

Note: Make sure to set the gasStipend amount accordingly.

Use the recommended forceSafeTransferETH function to transfer the ethers.

sherlock-admin commented 1 year ago

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

n33k commented:

invalid