Skip to content

Protocol Insolvency Risk Due to Unchecked Asset Increases in Atom and Triple Creation #81

Open
@hats-bug-reporter

Description

Github username: --
Twitter username: --
Submission hash (on-chain): 0x45af594e37907786efc19eea5046d4b897518936ba6dc1ee0e011e4b1bfa26dd
Severity: high

Description:
Description
In the EthMultiVault contract, there are two critical functions, _createTriple and _createAtom, that increase the total assets of vaults without ensuring that the protocol has sufficient ETH to back these increases. This can lead to a situation where the protocol becomes insolvent, unable to fulfill withdrawal requests if users attempt to redeem their shares.

  1. In _createTriple:

    • The function increases the total assets of each underlying atom vault by tripleConfig.atomDepositFractionOnTripleCreation / 3 without increasing the total shares or ensuring the protocol has these additional assets.
  2. In _createAtom:

    • The function deposits atomConfig.atomWalletInitialDepositAmount into the new atom vault for the atom wallet, without verifying if the protocol actually has these funds available.

These operations create a discrepancy between the recorded total assets and the actual ETH held by the protocol, potentially leading to insolvency.

Attack Scenario\

  1. An attacker observes that tripleConfig.atomDepositFractionOnTripleCreation is set to a non-zero value.
  2. The attacker creates multiple triples, each time increasing the total assets of the underlying atom vaults without actually providing these additional assets.
  3. Over time, this inflates the recorded total assets of the atom vaults beyond the actual ETH held by the protocol.
  4. When users try to withdraw their funds, the protocol may not have sufficient ETH to fulfill all withdrawal requests, leading to insolvency.

Similarly, for atom creation:

  1. An attacker creates multiple atoms, each time the protocol "deposits" atomConfig.atomWalletInitialDepositAmount into the new atom vault.
  2. If the protocol doesn't actually hold these funds, it creates a deficit that grows with each atom creation.
  3. This could be exploited to drain the protocol of its actual ETH holdings.

Attachments

// deposit atomWalletInitialDepositAmount amount of assets and mint the shares for the atom wallet
_depositOnVaultCreation(
id,
atomWallet, // receiver
atomConfig.atomWalletInitialDepositAmount

if (tripleConfig.atomDepositFractionOnTripleCreation > 0) {
for (uint256 i = 0; i < 3; i++) {
uint256 atomId = tripleAtomIds[i];
// increase the total assets in each underlying atom vault
_setVaultTotals(
atomId,
vaults[atomId].totalAssets + (tripleConfig.atomDepositFractionOnTripleCreation / 3),
vaults[atomId].totalShares
);
}

  1. Proof of Concept (PoC) File
function testProtocolInsolvencyRisk() external {
    vm.startPrank(alice, alice);

    // Create atoms
    uint256 testAtomCost = getAtomCost();
    uint256 subjectId = ethMultiVault.createAtom{value: testAtomCost}("subject");
    uint256 predicateId = ethMultiVault.createAtom{value: testAtomCost}("predicate");
    uint256 objectId = ethMultiVault.createAtom{value: testAtomCost}("object");

    // Create a triple
    uint256 tripleCost = getTripleCost();
    uint256 tripleId = ethMultiVault.createTriple{value: tripleCost}(subjectId, predicateId, objectId);

    vm.stopPrank();

    // Record initial balances
    uint256 initialProtocolBalance = address(ethMultiVault).balance;
    uint256 initialTotalAssets = vaultTotalAssets(subjectId) + vaultTotalAssets(predicateId) + vaultTotalAssets(objectId);

    // Set a non-zero atomDepositFractionOnTripleCreation
    vm.prank(address(ethMultiVault.generalConfig().admin));
    ethMultiVault.setAtomDepositFractionOnTripleCreation(1 ether);

    vm.startPrank(bob, bob);

    // Create another triple, which should increase atom vault balances
    ethMultiVault.createTriple{value: tripleCost}(subjectId, predicateId, objectId);

    vm.stopPrank();

    // Check final balances
    uint256 finalProtocolBalance = address(ethMultiVault).balance;
    uint256 finalTotalAssets = vaultTotalAssets(subjectId) + vaultTotalAssets(predicateId) + vaultTotalAssets(objectId);

    // Verify that recorded assets have increased more than actual ETH balance
    assertGt(finalTotalAssets - initialTotalAssets, finalProtocolBalance - initialProtocolBalance, "Protocol should show higher asset increase than actual ETH received");

    // Attempt to withdraw all assets from an atom vault
    vm.prank(alice);
    uint256 aliceShares = ethMultiVault.vaults(subjectId).balanceOf[alice];
    vm.expectRevert(); // Expect this to revert due to insufficient funds
    ethMultiVault.redeemAtom(aliceShares, alice, subjectId);
}
  1. Revised Code File (Optional)
contract EthMultiVault {
    uint256 public protocolOwnedAssets;

    function _createTriple(uint256 subjectId, uint256 predicateId, uint256 objectId, uint256 value) internal returns (uint256, uint256) {
        // ... existing code ...

        if (tripleConfig.atomDepositFractionOnTripleCreation > 0) {
            uint256 totalIncrease = tripleConfig.atomDepositFractionOnTripleCreation;
            require(protocolOwnedAssets >= totalIncrease, "Insufficient protocol assets");
            protocolOwnedAssets -= totalIncrease;

            for (uint256 i = 0; i < 3; i++) {
                uint256 atomId = tripleAtomIds[i];
                uint256 increase = totalIncrease / 3;
                _setVaultTotals(
                    atomId,
                    vaults[atomId].totalAssets + increase,
                    vaults[atomId].totalShares
                );
            }
        }

        // ... rest of the function ...
    }

    function _createAtom(bytes calldata atomUri, uint256 value) internal returns (uint256, uint256) {
        // ... existing code ...

        uint256 initialDeposit = atomConfig.atomWalletInitialDepositAmount;
        require(protocolOwnedAssets >= initialDeposit, "Insufficient protocol assets");
        protocolOwnedAssets -= initialDeposit;

        _depositOnVaultCreation(
            id,
            atomWallet,
            initialDeposit
        );

        // ... rest of the function ...
    }

    // New function to add protocol-owned assets
    function addProtocolAssets() external payable {
        protocolOwnedAssets += msg.value;
    }

    // ... other necessary changes ...
}

// Comments:
// 1. We've introduced a `protocolOwnedAssets` variable to track the protocol's actual ETH holdings.
// 2. Before increasing vault assets or making deposits, we check if the protocol has sufficient assets.
// 3. We deduct from `protocolOwnedAssets` when increasing vault balances or making initial atom wallet deposits.
// 4. A new function `addProtocolAssets` allows the protocol to add to its asset holdings.
// 5. This approach ensures that the protocol cannot become insolvent due to these operations.
// 6. Additional changes may be needed throughout the contract to maintain this balance accurately.

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinginvalidThis doesn't seem right

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions