DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Invalid

Gas griefing where complex fallback function in user contracts causes gas wastage vulnerability in addMigratedUnderlying function in the UnripdeFacet contract

Summary

The addMigratedUnderlying function transfers assets from a user to the protocol's contract using safeTransferFrom. A malicious user can implements a complex fallback function, the transaction can consume excessive gas, leading to significant gas costs for the protocol.

Impact: High

i: The protocol's gas costs can be significantly increased, leading to financial strain and operational inefficiencies.

ii: The issue can disrupt the protocol's normal operations.

Likelihood: Medium

i: The attack requires a malicious user to deploy a specially crafted contract but is feasible for a determined attacker.

Proof of Concept

1: A malicious user deploys a contract with a complex fallback function designed to consume excessive gas.

2: The malicious user interacts with the protocol, triggering addMigratedUnderlying to transfer assets from this malicious contract.

3: The complex fallback function is executed in the malicious contract, causing the transaction to consume excessive gas and leading to high gas costs for the protocol.

A malicious user can deploy a contract with a complex fallback function to increase gas consumption, exacerbating the gas wastage issue.

Malicious Contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract MaliciousContract {
IERC20 public immutable token;
constructor(IERC20 _token) {
token = _token;
}
// Complex fallback function to consume a lot of gas
fallback() external payable {
for (uint256 i = 0; i < 10000; i++) {
// Arbitrary computations to consume gas
uint256 temp = i * i;
}
}
// Function to receive tokens (does nothing or reverts)
function receiveTokens(uint256 amount) external {
// Revert the transaction to prevent tokens from being transferred out
revert("Tokens cannot be transferred out");
}
// Fallback function to prevent any Ether transfers
receive() external payable {
revert("Cannot receive Ether");
}
}

Test

TestAddMigratedUnderlying.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "../contracts/AddMigratedUnderlying.sol"; // Adjust the import path as needed
import "./MaliciousContract.sol";
contract TestAddMigratedUnderlying is Test {
AddMigratedUnderlying addMigratedUnderlying;
MaliciousContract maliciousContract;
IERC20 token;
function setUp() public {
token = new IERC20(); // Mock or real token contract
addMigratedUnderlying = new AddMigratedUnderlying();
maliciousContract = new MaliciousContract(token);
}
function testGasGriefingWithComplexFallback() public {
address user = address(0x1);
uint256 amount = 1000 * 10**18;
// Mint tokens to user
token.mint(user, amount);
// Approve AddMigratedUnderlying contract to transfer tokens
vm.prank(user);
token.approve(address(addMigratedUnderlying), amount);
// Transfer tokens to MaliciousContract with complex fallback
for (uint i = 0; i < 10; i++) {
vm.prank(user);
vm.expectRevert("Transfer failed or exceeded gas limit");
addMigratedUnderlying.addMigratedUnderlying(address(maliciousContract), amount);
}
}
}

Impact

1: The protocol incurs high gas costs for each transaction involving the complex fallback function, leading to increased operational expenses.

2: High gas consumption can disrupt normal protocol operations and reduce overall efficiency.

3: The cumulative gas costs from such transactions can lead to financial strain on the protocol.

Tools Used

Manual review

Recommendations

1: Implement checks to ensure that the gas usage of external calls does not exceed a reasonable limit.

2: Modify the addMigratedUnderlying function to attempt the transfer only once and handle failures gracefully without repeated attempts.

3: Implement rate limiting or throttling to reduce the potential for repeated gas-intensive transaction attempts.

function addMigratedUnderlying(
address unripeToken,
uint256 amount
) external payable fundsSafu noNetFlow noSupplyChange nonReentrant {
require(amount > 0, "Amount must be greater than zero");
LibDiamond.enforceIsContractOwner();
AppStorage storage s = LibAppStorage.diamondStorage();
// Check if the Unripe Token exists
require(s.sys.silo.unripeSettings[unripeToken].underlyingToken != address(0), "Unripe token not configured");
// Check if the contract has sufficient allowance
uint256 allowance = IERC20(s.sys.silo.unripeSettings[unripeToken].underlyingToken).allowance(LibTractor._user(), address(this));
require(allowance >= amount, "Insufficient token allowance");
// Transfer the underlying tokens from the caller to the contract
// Set a gas limit for the external call to prevent gas griefing
bool success;
(success, ) = address(s.sys.silo.unripeSettings[unripeToken].underlyingToken).call{gas: 50000}(
abi.encodeWithSignature(
"safeTransferFrom(address,address,uint256)",
LibTractor._user(),
address(this),
amount
)
);
require(success, "Transfer failed or exceeded gas limit");
// Update the underlying balance
LibUnripe.incrementUnderlying(unripeToken, amount);
// Emit an event (optional for tracking)
emit MigratedUnderlyingAdded(unripeToken, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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