Potential for Inaccurate Melting Due to Balance Discrepancies

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Furnace.sol#L65

Vulnerability details

Vulnerability Details

The core problem lies in the fact that the function uses lastPayoutBal to calculate the melt amount, but then updates lastPayoutBal based on the current balance. This can lead to inconsistencies if the actual balance has changed between melt operations.

Code Snippet

function melt() public {
    // ... (earlier parts of the function)

    uint256 amount = payoutRatio.mulu_toUint(lastPayoutBal);
    
    lastPayout += numPeriods;
    lastPayoutBal = rToken.balanceOf(address(this)) - amount;
    
    if (amount != 0) rToken.melt(amount);
}

The problem

1. amount is calculated based on lastPayoutBal.
2. lastPayoutBal is then updated using the current balance (rToken.balanceOf(address(this))).
3. If the actual balance has changed since the last melt (due to deposits or other operations), this new lastPayoutBal won't accurately reflect the starting balance for the next period.

Scenario

1. Initial lastPayoutBal: 1000 tokens
2. 500 tokens are deposited to the contract
3. melt() is called:
   • amount is calculated based on 1000 tokens
   • lastPayoutBal is updated to (1500 - amount), which doesn't accurately represent the starting balance for the next period

Impact

• The melting process may not accurately account for new deposits or withdrawals.
• Over time, this could lead to either over-melting or under-melting compared to the intended behavior.

Potential fix

To address this, the function should either:

1. Use the current balance to calculate the melt amount, or
2. Keep track of deposits/withdrawals between melt operations.

A possible implementation of the first approach:

function melt() public {
    // ... (earlier parts of the function)

    uint256 currentBalance = rToken.balanceOf(address(this));
    uint256 amount = payoutRatio.mulu_toUint(currentBalance);
    
    lastPayout += numPeriods;
    lastPayoutBal = currentBalance - amount;
    
    if (amount != 0) rToken.melt(amount);
}

This change ensures that melting is always based on the current balance, accounting for any changes since the last melt operation.

Assessed type

Context