022.md

Bitter Hemp Tiger

High

VVVVCInvestmentLedger does not account for price drop in invested amount

Summary

When users are investing in VVVVCInvestmentLedger, their invested amount is converted to Stable Coin's equivalent value. The contract also provides addInvestmentRecords function to add to total vested value in case of a exchange rate increase, however, a rate drop is not accounted, can cause unfair accounting for investors.

Root Cause

The following code snippet is from invest:

        // the stablecoin amount equivalent to the payment token amount supplied at the current exchange rate
        uint256 preFeeStableAmountEquivalent = (_params.amountToInvest * _params.exchangeRateNumerator) /
            exchangeRateDenominator;

        // the post-fee stableAmountEquivalent, to contribute toward user and round limits
        uint256 postFeeStableAmountEquivalent = preFeeStableAmountEquivalent -
            (preFeeStableAmountEquivalent * _params.feeNumerator) /
            FEE_DENOMINATOR;

        // check if kyc address has already invested the max stablecoin-equivalent amount for this round,
        // or if the total invested for this round has reached the limit
        if (
            postFeeStableAmountEquivalent > _params.kycAddressAllocation - kycAddressInvestedThisRound ||
            postFeeStableAmountEquivalent > _params.investmentRoundLimit - totalInvestedThisRound
        ) {
            revert ExceedsAllocation();
        }

        // update kyc address and total amounts invested for this investment round (in stablecoin terms)
        kycAddressInvestedPerRound[_params.kycAddress][
            _params.investmentRound
        ] += postFeeStableAmountEquivalent;
        totalInvestedPerRound[_params.investmentRound] += postFeeStableAmountEquivalent;

There is params.exchangeRateNumerator which according to comments, represents the current exchange rate relative to the stable coins, and postFeeStableAmountEquivalent would then represent the total value of vested amount in stable coins. This amount will be used to record total vested amount for user and each round.

On the other hand, in addInvestmentRecords:

    /**
        @notice Allows admin to add multiple investment records to the ledger
        @dev does not account for a nominal payment token / exchange rate - only modifies stablecoin equivalent invested
     */
    function addInvestmentRecords(
        address[] calldata _kycAddresses,
        uint256[] calldata _investmentRounds,
        uint256[] calldata _amountsToInvest
    ) external onlyAuthorized {
        if (
            _kycAddresses.length != _investmentRounds.length ||
            _investmentRounds.length != _amountsToInvest.length
        ) {
            revert ArrayLengthMismatch();
        }

        for (uint256 i = 0; i < _kycAddresses.length; i++) {
            address kycAddress = _kycAddresses[i];
            uint256 investmentRound = _investmentRounds[i];
            uint256 amountToInvest = _amountsToInvest[i];

            kycAddressInvestedPerRound[kycAddress][investmentRound] += amountToInvest;
            totalInvestedPerRound[investmentRound] += amountToInvest;
            emit VCInvestment(investmentRound, address(0), kycAddress, 0, 0, 0, amountToInvest);
        }
    }

Also according to the code comments, the function is meant to increase stable coin equivalent value when the price rises. However, it's also possible for the price to drop. In this case, records are not deducted. During a price drop event, this can cause recorded invested value more than actual invested value, counted in stable coin.

Internal pre-conditions

1. Alice invested 1000 X token, and according to the then exchange rate, it's equivalent to 500 stable coin, for example, USDC. Based on the record, the total vested worth is then 500 USDC.

External pre-conditions

1. The price drops, now the 1000 X token worth 400 USDC.

Attack Path

No response

Impact

The actual USDC worth of vested amount is only 400 now, but recorded as 500. There are no ways of reducing value, because such function is not implemented. This can cause unfair accounting, and potential loss of funds for the protocol.

PoC

No response

Mitigation

Consider using oracle to keep track of actual stable coin worth in real time.