Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential for Protocol Surplus Due to Indivisible atomDepositFractionOnTripleCreation #79

Open
hats-bug-reporter bot opened this issue Jun 30, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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

Description:
Description
In the setAtomDepositFractionOnTripleCreation function of the EthMultiVault contract, there's no check to ensure that the atomDepositFractionOnTripleCreation value is divisible by 3. This could lead to a small amount of ETH being left in the protocol when creating triples, as the value is meant to be equally distributed among three atoms.

Attack Scenario\

While not a direct security vulnerability, this oversight can lead to the following issues:

  1. Small amounts of ETH could accumulate in the protocol over time due to rounding errors.
  2. The actual distribution of ETH to the three atoms in a triple may be slightly uneven.
  3. Over a large number of transactions, this could result in a noticeable discrepancy between expected and actual ETH distribution.

Attachments

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "./EthMultiVault.sol";

contract EthMultiVaultFractionTest {
    EthMultiVault public vault;

    constructor(address _vaultAddress) {
        vault = EthMultiVault(_vaultAddress);
    }

    function testFractionDivisibility(uint256 newFraction) external {
        // Assume this contract has admin rights
        vault.setAtomDepositFractionOnTripleCreation(newFraction);
        
        uint256 setFraction = vault.tripleConfig().atomDepositFractionOnTripleCreation;
        uint256 remainder = setFraction % 3;
        
        require(remainder == 0, "Fraction not perfectly divisible by 3");
    }
}
  1. Revised Code File (Optional)
function setAtomDepositFractionOnTripleCreation(uint256 atomDepositFractionOnTripleCreation) external onlyAdmin {
    // Ensure the value is divisible by 3
    uint256 adjustedFraction = (atomDepositFractionOnTripleCreation / 3) * 3;
    
    tripleConfig.atomDepositFractionOnTripleCreation = adjustedFraction;

    emit AtomDepositFractionUpdated(adjustedFraction);
}
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 30, 2024
@mihailo-maksa mihailo-maksa added the invalid This doesn't seem right label Jul 1, 2024
@mihailo-maksa
Copy link
Collaborator

The report highlights that there is no check to ensure atomDepositFractionOnTripleCreation is divisible by 3, potentially leading to small amounts of ETH being left in the protocol.

Label: invalid

Comment:
Since the value of atomDepositFractionOnTripleCreation is fixed and can only be set by the admin, we do not consider this a security vulnerability. At best, it can be considered a low-priority enhancement, primarily since the accumulated difference will be so negligible that it doesn’t have any practical impact. No fix is needed if the value divisible by 3 is used in the deploy script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant