Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0xKann - Fee mismatch in invest() function leading to round limit breach #300

Open
sherlock-admin4 opened this issue Nov 17, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Nov 17, 2024

0xKann

Medium

Fee mismatch in invest() function leading to round limit breach

Summary

The invest() function in VVVVCInvestmentLedger.sol applies an off-chain signed fee only to the accounting values but not to the actual token transfer. This discrepancy allows an investment round to exceed its limit in real token value, despite appearing compliant in the contract's recorded data.

Root Cause

The root cause is the mismatch between how the fee is applied in the accounting logic versus the token transfer logic:

The fee-adjusted accounting value (postFeeStableAmountEquivalent) is used to update the ledger:

uint256 postFeeStableAmountEquivalent = 
    preFeeStableAmountEquivalent - (preFeeStableAmountEquivalent * _params.feeNumerator) / FEE_DENOMINATOR;

   kycAddressInvestedPerRound[_params.kycAddress][_params.investmentRound] += postFeeStableAmountEquivalent;
   totalInvestedPerRound[_params.investmentRound] += postFeeStableAmountEquivalent;
  1. However, the full investment amount is transferred to the contract, ignoring the fee:
  IERC20(_params.paymentTokenAddress).safeTransferFrom(
       msg.sender, address(this), _params.amountToInvest

As a result, while the accounting logic deducts the fee, the actual tokens transferred to the contract do not reflect this deduction.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Setup

  1. Investment Round Limit: 900 USDC (in stablecoin equivalent).
  2. Parameters Provided by Admin:
    • amountToInvest: 1100 tokens
    • feeNumerator: 1000 (10% fee)
    • exchangeRateNumerator/Denominator: 1:1 (1 token = 1 USDC).
  3. User Calls invest() with these parameters.

Execution

  1. Contract computes the pre-fee accounting value:

    uint256 preFeeStableAmountEquivalent = (_params.amountToInvest * _params.exchangeRateNumerator) /
        exchangeRateDenominator;
    
    // Pre-fee value
    preFeeStableAmountEquivalent = (1100 * 1) / 1 = 1100 USDC
  2. Contract calculates the post-fee accounting value:

    uint256 postFeeStableAmountEquivalent = preFeeStableAmountEquivalent -
        (preFeeStableAmountEquivalent * _params.feeNumerator) / FEE_DENOMINATOR;
    
    // Post-fee value
    postFeeStableAmountEquivalent = 1100 - (1100 * 1000) / 10_000 = 990 USDC
  3. Investment checks pass because 990 USDC <= 900 USDC round limit.

  4. Contract updates the accounting values:

    kycAddressInvestedPerRound[_params.kycAddress][_params.investmentRound] += postFeeStableAmountEquivalent;
    totalInvestedPerRound[_params.investmentRound] += postFeeStableAmountEquivalent;
    
    // Updated values
    kycAddressInvestedPerRound[_params.kycAddress][_params.investmentRound] = 990 USDC;
    totalInvestedPerRound[_params.investmentRound] = 990 USDC;
  5. However, the full amount is transferred to the contract:

    IERC20(_params.paymentTokenAddress).safeTransferFrom(
        msg.sender, address(this), _params.amountToInvest
    );
    
    // Transferred amount
    _params.amountToInvest = 1100 tokens

Outcome

  • Accounting values show 990 USDC invested.
  • Actual token balance in the contract is 1100 tokens (equivalent to 1100 USDC).
  • The investment round effectively holds 1100 USDC worth of tokens, exceeding the 900 USDC limit by 200 USDC.

Impact

Limit Violation:
Exceeds the defined investment round limits.
Breaches governance rules for investment caps.
Trust Breach:
Creates a discrepancy between stated and actual behavior.
Undermines user trust and compliance with the stated terms.
Exploitation Potential:
A malicious user could inflate the round's holdings by exploiting this mismatch.

PoC

No response

Mitigation

Adjust the invest() function to transfer the fee-adjusted token amount instead of the full amount:

uint256 adjustedAmountToTransfer = (_params.amountToInvest * (FEE_DENOMINATOR - _params.feeNumerator)) /
    FEE_DENOMINATOR;

IERC20(_params.paymentTokenAddress).safeTransferFrom(
    msg.sender,
    address(this),
    adjustedAmountToTransfer
);
@sherlock-admin3 sherlock-admin3 changed the title Tricky Mahogany Vulture - Fee mismatch in invest() function leading to round limit breach 0xKann - Fee mismatch in invest() function leading to round limit breach Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant