Squilliam - [M-16] `FixedLib.sol` - Arithmetic Overflows could lead to DoS (Denial of Service)

[sherlock-admin4]

Squilliam

medium

[M-16] FixedLib.sol - Arithmetic Overflows could lead to DoS (Denial of Service)

Summary

The calculateDeposit function in the FixedLib library is vulnerable to potential arithmetic overflows, which could lead to unexpected behavior, incorrect calculations, or denial of service (DoS) conditions. The testCalculateDepositOverflow test demonstrates this vulnerability by setting up a FixedLib.Pool struct with a very large unassignedEarnings value and expecting the function to revert due to an overflow.

Vulnerability Detail

Add the following test to FixedLib.t.sol:

function testCalculateDepositOverflow() external {
    FixedLib.Pool memory pool = FixedLib.Pool({
        borrowed: 1e18,
        supplied: 0,
        unassignedEarnings: type(uint256).max,
        lastAccrual: 0
    });

    uint256 amount = 1e18;
    uint256 backupFeeRate = 1e18;

    vm.expectRevert();
    (uint256 yield, uint256 backupFee) = pool.calculateDeposit(amount, backupFeeRate);
}

Here's a simplified walkthrough of the test:

The test creates a FixedLib.Pool struct with specific values, including a very large unassignedEarnings value set to type(uint256).max, which represents the maximum value that can be stored in a uint256 variable. It sets the amount and backupFeeRate variables to 1e18. The test uses vm.expectRevert() to expect a revert to occur in the subsequent function call. Finally, it calls the calculateDeposit function with the provided amount and backupFeeRate, expecting it to revert due to an overflow.

Run this test with forge test --mt testCalculateDepositOverflow

When you run this test, it will pass, indicating that the calculateDeposit function is vulnerable to arithmetic overflows and does not handle large values correctly, potentially leading to DoS conditions.

Impact

The calculateDeposit function does not handle large values correctly and is susceptible to arithmetic overflows, it could result in the following impacts:

Incorrect calculation of yield and backup fees, leading to financial discrepancies and potential loss of funds.

Denial of service (DoS) conditions if the overflow causes the function to revert permanently, rendering the pool unusable.

Exploitation by malicious actors who could manipulate the pool's state or earn unintended profits by leveraging the overflow vulnerability.

Code Snippet

The calculateDeposit function in FixedLib.sol can be found here: https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/main/protocol/contracts/utils/FixedLib.sol?plain=1#L18-L29

Tool used

Foundry Manual Review

Recommendation

To mitigate the potential DoS vulnerability in the calculateDeposit function, the following steps should be taken:

Implement proper overflow checks and safe math operations to handle large values gracefully and prevent unexpected behavior.

Consider using libraries like OpenZeppelin's SafeMath or Solidity's built-in overflow protection (in Solidity 0.8.0 and above) to perform arithmetic operations securely.

Implement comprehensive error handling and revert mechanisms to gracefully handle exceptional scenarios and prevent permanent DoS conditions.

[santipu03]

This issue is invalid because it won't happen. No active tokens have a supply big enough to cause the value of unassignedEarnings to even slightly approach type(uint256).max.