Free Ether will get printed because of incorrectly handing Fees on `depositTriple()`

[hats-bug-reporter[bot]]

Github username: @Al-Qa-qa Twitter username: al_qa_qa Submission hash (on-chain): 0x590efc11e1735caefc941a4b813422392ade726f0329ac1117feaa01cc1b8388 Severity: high

Description: Description\ in depositTriple(), if we checked the code we will see the following.

    function depositTriple( ... ) ... {
        ...

@>      uint256 protocolFee = protocolFeeAmount(msg.value, id);
        uint256 userDepositAfterprotocolFee = msg.value - protocolFee;

        // deposit eth into vault and mint shares for the receiver
@>      uint256 shares = _deposit(receiver, id, userDepositAfterprotocolFee);

        _transferFeesToProtocolVault(protocolFee);

        // distribute atom shares for all 3 atoms that underly the triple
        uint256 atomDepositFraction = atomDepositFractionAmount(userDepositAfterprotocolFee, id);
@>      _depositAtomFraction(id, receiver, atomDepositFraction);

        return shares;
    }

1- We get the Protocol Fees

2. deposit amount - fees to the receiver on Triple Vault
3. deposit atomFraction

The issue here, is that we are not subtracting atomDepositFraction from the amount before depositing to the receiver.

• Caller deposited 1000 || protocolFees = 100 || atomDepositFraction = 50
• Caller will receiver 900
• Protocol Will receive 100
• Atom Vaults will receive 50.
• Total = 900 + 100 + 50 = 1050 > 1000

So there is free 50 ETH got deposited to Atom vaults, without any sending. This occurs before we are minting the hole amount to the receiver Total - protocolFees, without subtracting the Fees that will goes to Atom Vaults. Leading to inflation and free minting.

if we checked _createTriple() we will find that it handles the case correctly.

EthMultiVault.sol#L628-L635

        uint256 atomDepositFraction = atomDepositFractionAmount(userDepositAfterprotocolFee, id);

        // give the user shares in the positive triple vault
        _depositOnVaultCreation(
            id,
            msg.sender, // receiver
@>          userDepositAfterprotocolFee - atomDepositFraction
        );

As we can see, we we minting depositAfter taking fees subtracting the atomDepositFraction. so the problem lies on depositTriple()

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

• This Test will test creating Triple Vault with Depositing 10 ETH

  function testAuditor__createTripleVault__checkAssetsDeposited() external {
      vm.startPrank(alice, alice);
  
      // Depositing 10 ether
      uint256 testDepositAmountTriple = 10 ether + getTripleCost();
  
      // execute interaction - create atoms
      uint256 subjectId = ethMultiVault.createAtom{value: getAtomCost()}("subject");
      uint256 predicateId = ethMultiVault.createAtom{value: getAtomCost()}("predicate");
      uint256 objectId = ethMultiVault.createAtom{value: getAtomCost()}("object");
  
      uint256[3] memory totalAssetsBeforeAtomVaults =
          [vaultTotalAssets(subjectId), vaultTotalAssets(predicateId), vaultTotalAssets(objectId)];
  
      // execute interaction - create triples
      uint256 id = ethMultiVault.createTriple{value: testDepositAmountTriple}(subjectId, predicateId, objectId);
  
      console2.log("Tripe Vault Total Assets Before: 0");
      console2.log("Atom Vault[0] Total Assets Before:", totalAssetsBeforeAtomVaults[0]);
      console2.log("Atom Vault[1] Total Assets Before:", totalAssetsBeforeAtomVaults[1]);
      console2.log("Atom Vault[2] Total Assets Before:", totalAssetsBeforeAtomVaults[2]);
      console2.log("---------------");
      console2.log("Tripe Vault Total Assets:", vaultTotalAssets(id));
      console2.log("Atom Vault[0] Total Assets:", vaultTotalAssets(subjectId));
      console2.log("Atom Vault[1] Total Assets:", vaultTotalAssets(predicateId));
      console2.log("Atom Vault[2] Total Assets:", vaultTotalAssets(objectId));
      console2.log("---------------");
      uint256 totalOldAssets = totalAssetsBeforeAtomVaults[0] + totalAssetsBeforeAtomVaults[1] + totalAssetsBeforeAtomVaults[2];
      uint256 totalNewAssets = vaultTotalAssets(id) + vaultTotalAssets(subjectId) + vaultTotalAssets(predicateId) + vaultTotalAssets(objectId);
      console2.log("Total Old Assets:", totalOldAssets);
      console2.log("Total New Assets:", totalNewAssets);
      console2.log("New Minted Assets:", totalNewAssets - totalOldAssets);
  
      vm.stopPrank();
  }

• This test will test Depositing to Triple with 10 ETH

    function testAuditor__DepositTriple__checkAssetsDeposited() external {
        vm.startPrank(alice, alice);

        // test values
        uint256 testAtomCost = getAtomCost();
        uint256 testDepositAmount = 10 ether;

        // execute interaction - create atoms
        uint256 subjectId = ethMultiVault.createAtom{value: testAtomCost}("subject");
        uint256 predicateId = ethMultiVault.createAtom{value: testAtomCost}("predicate");
        uint256 objectId = ethMultiVault.createAtom{value: testAtomCost}("object");

        // execute interaction - create a triple using test deposit amount for triple (0.01 ether)
        uint256 id = ethMultiVault.createTriple{value: getTripleCost()}(subjectId, predicateId, objectId);

        vm.stopPrank();

        // snapshots before interaction
        uint256 totalAssetsBefore = vaultTotalAssets(id);

        uint256[3] memory totalAssetsBeforeAtomVaults =
            [vaultTotalAssets(subjectId), vaultTotalAssets(predicateId), vaultTotalAssets(objectId)];

        vm.startPrank(bob, bob);

        // execute interaction - deposit atoms
        ethMultiVault.depositTriple{value: testDepositAmount}(address(1), id);

        // execute interaction - deposit triple into counter vault
        uint256 counterId = ethMultiVault.getCounterIdFromTriple(id);

        ethMultiVault.depositTriple{value: testDepositAmount}(address(2), counterId);

        console2.log("Tripe Vault Total Assets Before:", totalAssetsBefore);
        console2.log("Atom Vault[0] Total Assets Before:", totalAssetsBeforeAtomVaults[0]);
        console2.log("Atom Vault[1] Total Assets Before:", totalAssetsBeforeAtomVaults[1]);
        console2.log("Atom Vault[2] Total Assets Before:", totalAssetsBeforeAtomVaults[2]);
        console2.log("---------------");
        console2.log("Tripe Vault Total Assets:", vaultTotalAssets(id));
        console2.log("Atom Vault[0] Total Assets:", vaultTotalAssets(subjectId));
        console2.log("Atom Vault[1] Total Assets:", vaultTotalAssets(predicateId));
        console2.log("Atom Vault[2] Total Assets:", vaultTotalAssets(objectId));
        console2.log("---------------");
        uint256 totalOldAssets = totalAssetsBeforeAtomVaults[0] + totalAssetsBeforeAtomVaults[1] + totalAssetsBeforeAtomVaults[2];
        uint256 totalNewAssets = vaultTotalAssets(id) + vaultTotalAssets(subjectId) + vaultTotalAssets(predicateId) + vaultTotalAssets(objectId);
        console2.log("Total Old Assets:", totalOldAssets);
        console2.log("Total New Assets:", totalNewAssets);
        console2.log("New Minted Assets:", totalNewAssets - totalOldAssets);
        vm.stopPrank();
    }

You will find that in case of creating, the new minted tokens is less than 10 ETH with a little amount, because of protocol fees.

And in case of depositTriple(), you will find that the new minted ETH is like 11 ETH.

Recommendations

Subtract Atom Fees before minting to the receiver in depositTriple().

        uint256 protocolFee = protocolFeeAmount(msg.value, id);
        uint256 userDepositAfterprotocolFee = msg.value - protocolFee;

+      uint256 atomDepositFraction = atomDepositFractionAmount(userDepositAfterprotocolFee, id);

        // deposit eth into vault and mint shares for the receiver
-       uint256 shares = _deposit(receiver, id, userDepositAfterprotocolFee);
+       uint256 shares = _deposit(receiver, id, userDepositAfterprotocolFee - atomDepositFraction);

        _transferFeesToProtocolVault(protocolFee);

        // distribute atom shares for all 3 atoms that underly the triple
-       uint256 atomDepositFraction = atomDepositFractionAmount(userDepositAfterprotocolFee, id);
       _depositAtomFraction(id, receiver, atomDepositFraction);

[Al-Qa-qa]

This is Invalid, no need to read it.

[mihailo-maksa]

The report is invalid. The current implementation of depositTriple accurately handles the calculation and distribution of protocol fees and atom deposit fractions. The confusion arises from the sequence of function calls and the internal logic used to manage these fees.

In depositTriple, the protocol fee is calculated and subtracted from the user's deposit to determine the net amount to be deposited. This net amount is then used to mint shares for the user in the triple vault. The atomDepositFraction is subsequently distributed to the underlying atom vaults through the _depositAtomFraction function.

The internal function getDepositSharesAndFees is responsible for calculating the necessary shares and fees, ensuring that all amounts are correctly accounted for before any shares are minted. This comprehensive approach prevents any scenario where "free" Ether would be minted or where the total assets would exceed the intended amounts.

Thus, the protocol correctly manages all deposits, fees, and share distributions, ensuring accurate accounting and preventing any unintended inflation or discrepancies. The current implementation is sound and does not require changes based on the reported issue, as also acknowledged by the reporter.