#36309 [SC-Low] TimeBasedCollateralPool: After _resetPool gets called (internally) a depositor can break most functionalities of the smart contract
Submitted on Oct 28th 2024 at 22:48:59 UTC by @max10afternoon for Audit Comp | Anvil
Report ID: #36309
Report Type: Smart Contract
Report severity: Low
Target: https://etherscan.io/address/0xd042C267758eDDf34B481E1F539d637e41db3e5a
Impacts:
Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Description
Brief/Intro
After a pool gets reset, a depositor can call unstake to intentionally (or unintentionally) break almost all functionalities of the smart contract, including staking and unstaking. Leading to a freezing of funds in cases were and ExitBalance is present, or to a general disruption of service otherwise.
Vulnerability Details
When a pool gets reset the associated reservation ID gets set to 0, together with any pending epoch: ``` function _resetPool(address _tokenAddress) internal { // Note: we are not calling _unlockEligibleTokenContractPendingUnstakes because it can call this function.
``` This two facts will be relevant inside of the '_unlockEligibleTokenContractPendingUnstakes', which gets called by all stake functionalities, unstake and claim. As calling unstake after the pool got reset will cause _unlockEligibleTokenContractPendingUnstakes to revert braking all of the above functionalities, here is why:
As you may notice at the begging, if uint256 firstEpoch = contractStateStorage.firstPendingUnstakeEpoch; is 0, no further computations will be made, avoiding any unexpected revert, that would come from interacting with the underlying Collateral contract trough a reservation with ID equal to 0. That said a user can call unstake to froce the contractStateStorage.firstPendingUnstakeEpoch to be greater than 0**, meaning that after the epoch elapses, the above mentioned check will pass and, unless someone have deposited before the time has passed, every functionality dependent on _unlockEligibleTokenContractPendingUnstakes will revert, breaking almost every functionalities of the contract with respect to the targeted token and freezing any funds found in a potential ExitBalance allocation until the implementation gets re deployed (with the owner's time lock currently having a min delay of 7 days).
**Note that the check performed inside of unstake, to verify if the account was reset will be ignored, as when epoch is equal to 0, the _poolWasReset variable will be set to false, meaning that accountWasReset will also be set to false, allowing the execution to reach the critical _addToContractPendingUnstakeNextEpoch function.
All of this can be done maliciously do damage the users and owners of the protocol (even by frontrunning the pool's reset with a dust amount deposit, see PoC) or by mistake by a user trying to legitimately call unstake.
Impact Details
Griefing: A malicious user can decide to break most functionalities of the contract either by having some funds staked already of by frontrunning (for almost no cost). Also this might lead to a temporary freezing of funds in cases were an ExitBalance was allocated.
Proof of Concept
Here is a simple foundry PoC interacting directly with the in scope assets (0xd042C267758eDDf34B481E1F539d637e41db3e5a) after properly initializing it, any interaction trough a proxy would lead to the same results.
Place your alchemy API key in the URL on line 119 of the script: ``` // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.0;
import "forge-std/Test.sol"; import "forge-std/console.sol";
interface IStruct{
} interface IERC20 {
}
interface ICollateral is IStruct{
}
interface ITimeBasedCollateralPool is IStruct {