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

Zero Amount Handling in addMigratedUnderlying Leading to Incorrect Calculations in chop Function

Summary

The addMigratedUnderlying function in the UnripeFacet is vulnerable to scenarios where zero amounts can be processed, leading to incorrect underlying balances. This impacts the chop function, which relies on accurate underlying balances to calculate ripe tokens.

Vulnerability Details

The addMigratedUnderlying function does not check if the amount being processed is zero. This leads to scenarios where the underlying balance is not incremented as expected, which subsequently affects calculations in the chop function that depend on accurate underlying balances.

Proof of concept

Step-by-Step Scenario:

1: A user or function calls addMigratedUnderlying with a zero amount.

2: The incrementUnderlying function is called, but since the amount is zero, the balance remains unchanged.

3: The chop function calculates the amount of ripe tokens based on the unchanged underlying balance, leading to users receiving fewer ripe tokens than expected.

Test

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "forge-std/Test.sol";
import "../contracts/AddMigratedUnderlying.sol"; // Adjust the import path as needed
contract TestAddMigratedUnderlying is Test {
AddMigratedUnderlying addMigratedUnderlying;
IERC20 token;
function setUp() public {
token = new IERC20(); // Mock or real token contract
addMigratedUnderlying = new AddMigratedUnderlying();
}
function testAddMigratedUnderlyingZeroAmount() public {
address user = address(0x1);
uint256 amount = 0;
// Mint tokens to user
token.mint(user, amount);
// Approve AddMigratedUnderlying contract to transfer tokens
vm.prank(user);
token.approve(address(addMigratedUnderlying), amount);
// Attempt to add migrated underlying with zero amount
vm.prank(user);
vm.expectRevert("Amount must be greater than zero");
addMigratedUnderlying.addMigratedUnderlying(address(token), amount);
}
}

For clarification

The chop function relies on the accurate calculation of the underlying balance to determine how many ripe tokens a user should receive when they "chop" their unripe tokens. Here’s an example to illustrate the impact of a zero amount:

1: Initial State:

  • Total Unripe Tokens: 100,000

  • Underlying Balance: 50,000 tokens

2: Expected Increment:

  • A call to addMigratedUnderlying is made with an amount of 10,000 tokens.

  • The underlying balance should increase to 60,000 tokens.

3: Actual Increment with Zero Amount:

  • If the call to addMigratedUnderlying is made with a zero amount:

  • The underlying balance remains at 50,000 tokens.

4: Impact on chop Calculation:

  • Suppose a user chops 1,000 unripe tokens.

  • Expected Calculation:

i: New Underlying Balance: 60,000 tokens

ii: Ripe Tokens per Unripe Token: 60,000 / 100,000 = 0.6 ripe tokens

iii: User should receive: 1,000 * 0.6 = 600 ripe tokens

  • Actual Calculation with Zero Amount:

i: New Underlying Balance: 50,000 tokens

ii: Ripe Tokens per Unripe Token: 50,000 / 100,000 = 0.5 ripe tokens

iii: User receives: 1,000 * 0.5 = 500 ripe tokens

4: The user receives 100 fewer ripe tokens than expected.

Impact

  • Users will receive fewer ripe tokens than they should, leading to financial discrepancies.

  • Incorrect token distributions will cause dissatisfaction among users, potentially harming the protocol's reputation.

  • Processing zero amounts incurs unnecessary gas costs and emits irrelevant events, leading to operational inefficiencies.

Tools Used

Manual review

Recommendations

1: Implement a check for zero amounts in the addMigratedUnderlying function to ensure that only positive amounts are processed.

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
bool success = IERC20(s.sys.silo.unripeSettings[unripeToken].underlyingToken).transferFrom(
LibTractor._user(),
address(this),
amount
);
require(success, "Transfer failed");
// 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: Non-acceptable severity

Support

FAQs

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