Inconsistent State Management in tripleAtomShares Mapping

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x301f2aa465bc5205e1d38f7d64769bc92a3ca07464f32503718d3f282577bbfd Severity: medium

Description: Description\ In the EthMultiVault contract, the _depositAtomFraction function updates a tripleAtomShares mapping to track shares of atoms that are part of a triple. However, this mapping is not consistently updated across all relevant operations in the protocol. Specifically:

1. Direct deposits to an atom that is part of a triple do not update this mapping.
2. Redemptions of triples or individual atoms that are part of triples do not decrease the shares in this mapping.

This inconsistency can lead to an invalid protocol state and cause problems with interoperability between atoms and triples.

Attack Scenario\ While not directly exploitable, this issue can lead to the following problems:

1. A user deposits directly into an atom that is part of a triple. Later, when trying to redeem the triple, the protocol may not accurately represent the user's true share of the atom within the triple context.

2. A user redeems shares directly from an atom that is part of a triple. The tripleAtomShares mapping will now overstate the user's share of the atom within the triple, potentially allowing them to redeem more from the triple than they should be able to.

3. Over time, as users interact with atoms both directly and as part of triples, the discrepancy between the actual shares and what's recorded in tripleAtomShares grows, leading to increasingly inaccurate calculations and potential fund lockups or unfair distributions.

Attachments

1. Proof of Concept (PoC) File

   function testInconsistentShares(uint256 tripleId, uint256 atomId, uint256 amount) external {
       // Assume atomId is part of tripleId
   
       // Direct deposit to atom
       vault.depositAtom{value: amount}(address(this), atomId);
   
       // Check tripleAtomShares
       uint256 recordedShares = vault.tripleAtomShares(tripleId, atomId, address(this));
       uint256 actualShares = vault.vaults(atomId).balanceOf[address(this)];
   
       // This assertion will likely fail, showing the inconsistency
       assert(recordedShares == actualShares);
   
       // Now redeem from the atom
       uint256 sharesToRedeem = actualShares / 2;
       vault.redeemAtom(sharesToRedeem, address(this), atomId);
   
       // Check tripleAtomShares again
       recordedShares = vault.tripleAtomShares(tripleId, atomId, address(this));
       actualShares = vault.vaults(atomId).balanceOf[address(this)];
   
       // This assertion will fail, showing that redemption doesn't update tripleAtomShares
       assert(recordedShares == actualShares);
   }

2. Revised Code File (Optional)

   • add an updateTripleAtomShares function to handle updating the tripleAtomShares mapping.
   • This function is called in both depositAtom and redeemAtom to ensure consistency.
   • It iterates through all triples to find those that contain the atom being deposited to or redeemed from.
   • The isDeposit parameter determines whether to add or subtract shares.
   • Note that this solution increases gas costs for atom operations. Consider optimizing or using a different data structure for production.

[mihailo-maksa]

The report highlights an inconsistency in updating the tripleAtomShares mapping during certain operations, which could lead to an invalid protocol state.

Label: invalid

Comment: The report is invalid since tripleAtomShares is designed to track the amount of shares users proportionally receive on triple deposits. While this mapping does provide users with a proportional amount of shares in each underlying atom, the opposite functionality is not intended in redeemTriple. We have considered and abandoned this functionality due to various constraints and edge cases. Claims of potential "invalid protocol state" are invalid as tripleAtomShares is only used for tracking purposes and does not directly affect the protocol state. For further understanding, please refer to issue #48.