DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Submission Details
Severity: low
Invalid

[H] Users who interact with UnwrapAndSendETH can experience loss of funds and DOS attacks

Summary

The UnwrapAndSendETH contract is designed to receive WETH and unwrap it into ETH before sending it to a specified address.

Its a helper contract for Pipeline, although it can also be used by other contracts & users.

However, there are several issues:

The contract doesn't validate the to address, it lacks access control and it doesn't handle data properly if a contract
sends data with the WETH.

Additionally, there is a dev comment "
/// @dev Make sure to load WETH into this contract before calling this function
"

Which suggests there will be a balance of WETH in the contract before the unwrap function is called.

Vulnerability Details

OK first, lets ascertain the importance of validating the to address. If the to address is a contract,
and its assumed the recipient is an EOA, then sending native tokens in this fashion is fine. However,
if the recipient is a contract, then the contract must be able to handle the incoming ETH. This is nullified
if the contract for example has been destroyed via self-destruct or not deployed. The call will return successful
even if no transfer was executed. This will lead to a loss of funds.

Ref the below Uniswap report under report 9.

https://github.com/Uniswap/v3-core/blob/main/audits/tob/audit.pdf

Another issue to be weary of is a returnbomb of gas placed on the UnwrapAndSendETH contract. Without going into the
technical details of this, this essentially means that the to address can return a huge amount of data once invoked
by the low level .call method, which gets copied into memory. This can lead to a DOS attack.

Here is an example below

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;
/**
* @title Returnbomb attack example
*/
contract Evil {
uint256 public counter;
function startBomb() external returns (bytes memory) {
++counter;
// solhint-disable-next-line no-inline-assembly
assembly {
revert(0, 10000)
}
}
}
contract Victim {
function oops() public returns (bool, bytes memory) {
Evil evil = new Evil();
/// @dev If you put 3396 gas, the subcall will revert with an OOG error.
(bool success, bytes memory returnData) =
// solhint-disable-next-line avoid-low-level-calls
address(evil).call{gas: 3397}(abi.encodeWithSelector(evil.startBomb.selector));
return (success, returnData);
}
}

This can be mitigated by using this library for example:
https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol

The contract itself lacks access control, which means once, according to the dev comment, WETH is
loaded into the contract, an attacker can drain the entire balance, simply by viewing the balance of
address(this) before calling the unwrap function.

function unwrapAndSendETH(address to) external

Finally, the contract doesn't handle data in transactions. If data is sent through to the UnwrapAndSendETH function from the
Pipeline contract for example or even directly from another smart contract, the UnwrapAndSendETHcontract is designed to only receive
ETH via the receive() external payable {}.

If a contract needs to send calldata, you should facilitate this by implementing the fallback function, as detailed
in the solidity docs.

https://docs.soliditylang.org/en/v0.8.9/contracts.html?highlight=receive#receive-ether-function

Impact

Individually each of these components can lead to loss of funds due to lack of
contract existence or direct draining of funds, plus the additional DOS via returnbomb.

Tools Used

Manual Review

Recommendations

To deal with the lack of validation of the to address, you can implement a check to ensure the contract
is not self-destructed or not deployed. Make sure its also not the address(0).

Use the https://github.com/nomad-xyz/ExcessivelySafeCall/blob/main/src/ExcessivelySafeCall.sol
library to deal with the potential of a returnbomb gas attack.

Place appropriate access controls to prevent unauthorized access to the contract.

Consider implementing a fallback function to handle calldata in transactions with sending ETH.

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Pipeline access control

Informational/Invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.