mirror of
https://github.com/vacp2p/staking-reward-streamer.git
synced 2026-01-09 21:18:01 -05:00
test(RewardsStreamerMP): add test to ensure funds are saffe when stack overflow
If there's a malicious upgrade which causes a stack overflow error when `leave()` is called, the user of the vault should still be able to get their funds out. This commit adds a test that proofs this is happening.
This commit is contained in:
r4bbit
61
.gas-report
61
.gas-report
@@ -22,27 +22,27 @@
|
||||
| MIN_LOCKUP_PERIOD | 274 | 274 | 274 | 274 | 11 |
|
||||
| MP_RATE_PER_YEAR | 230 | 230 | 230 | 230 | 3 |
|
||||
| SCALE_FACTOR | 294 | 294 | 294 | 294 | 41 |
|
||||
| STAKING_TOKEN | 2381 | 2381 | 2381 | 2381 | 194 |
|
||||
| accountedRewards | 373 | 951 | 373 | 2373 | 76 |
|
||||
| STAKING_TOKEN | 2381 | 2381 | 2381 | 2381 | 198 |
|
||||
| accountedRewards | 373 | 970 | 373 | 2373 | 77 |
|
||||
| emergencyModeEnabled | 2398 | 2398 | 2398 | 2398 | 7 |
|
||||
| enableEmergencyMode | 2460 | 19367 | 24652 | 24652 | 8 |
|
||||
| getAccount | 1621 | 1621 | 1621 | 1621 | 72 |
|
||||
| getStakedBalance | 2567 | 2567 | 2567 | 2567 | 1 |
|
||||
| initialize | 137773 | 137773 | 137773 | 137773 | 50 |
|
||||
| isTrustedCodehash | 563 | 1078 | 563 | 2563 | 194 |
|
||||
| initialize | 137773 | 137773 | 137773 | 137773 | 51 |
|
||||
| isTrustedCodehash | 563 | 1078 | 563 | 2563 | 198 |
|
||||
| leave | 62061 | 62061 | 62061 | 62061 | 1 |
|
||||
| lock | 9839 | 33584 | 14168 | 76747 | 3 |
|
||||
| proxiableUUID | 363 | 363 | 363 | 363 | 3 |
|
||||
| rewardIndex | 394 | 420 | 394 | 2394 | 76 |
|
||||
| setTrustedCodehash | 26226 | 26226 | 26226 | 26226 | 50 |
|
||||
| stake | 134677 | 171329 | 176279 | 196758 | 60 |
|
||||
| totalMP | 329 | 329 | 329 | 329 | 79 |
|
||||
| totalMaxMP | 373 | 373 | 373 | 373 | 79 |
|
||||
| totalStaked | 329 | 329 | 329 | 329 | 80 |
|
||||
| rewardIndex | 394 | 419 | 394 | 2394 | 77 |
|
||||
| setTrustedCodehash | 26226 | 26226 | 26226 | 26226 | 51 |
|
||||
| stake | 134677 | 171411 | 176279 | 196758 | 61 |
|
||||
| totalMP | 329 | 329 | 329 | 329 | 80 |
|
||||
| totalMaxMP | 373 | 373 | 373 | 373 | 80 |
|
||||
| totalStaked | 329 | 329 | 329 | 329 | 81 |
|
||||
| unstake | 63997 | 84793 | 63997 | 120464 | 13 |
|
||||
| updateAccountMP | 15353 | 17591 | 17855 | 17855 | 19 |
|
||||
| updateGlobalState | 11066 | 41310 | 30530 | 63481 | 28 |
|
||||
| upgradeToAndCall | 3124 | 8887 | 10809 | 10809 | 4 |
|
||||
| upgradeToAndCall | 3124 | 9263 | 10809 | 10809 | 5 |
|
||||
|
||||
|
||||
| src/StakeManagerProxy.sol:StakeManagerProxy contract | | | | | |
|
||||
@@ -55,22 +55,22 @@
|
||||
| MIN_LOCKUP_PERIOD | 701 | 3973 | 5201 | 5201 | 11 |
|
||||
| MP_RATE_PER_YEAR | 657 | 657 | 657 | 657 | 3 |
|
||||
| SCALE_FACTOR | 721 | 721 | 721 | 721 | 41 |
|
||||
| STAKING_TOKEN | 7308 | 7308 | 7308 | 7308 | 194 |
|
||||
| accountedRewards | 800 | 1378 | 800 | 2800 | 76 |
|
||||
| STAKING_TOKEN | 7308 | 7308 | 7308 | 7308 | 198 |
|
||||
| accountedRewards | 800 | 1397 | 800 | 2800 | 77 |
|
||||
| emergencyModeEnabled | 7325 | 7325 | 7325 | 7325 | 7 |
|
||||
| enableEmergencyMode | 28455 | 45356 | 50640 | 50640 | 8 |
|
||||
| getAccount | 2075 | 2075 | 2075 | 2075 | 72 |
|
||||
| getStakedBalance | 7497 | 7497 | 7497 | 7497 | 1 |
|
||||
| implementation | 343 | 935 | 343 | 2343 | 277 |
|
||||
| isTrustedCodehash | 993 | 1508 | 993 | 2993 | 194 |
|
||||
| rewardIndex | 821 | 847 | 821 | 2821 | 76 |
|
||||
| setTrustedCodehash | 52872 | 52872 | 52872 | 52872 | 50 |
|
||||
| totalMP | 756 | 756 | 756 | 756 | 79 |
|
||||
| totalMaxMP | 800 | 800 | 800 | 800 | 79 |
|
||||
| totalStaked | 756 | 756 | 756 | 756 | 80 |
|
||||
| implementation | 343 | 936 | 343 | 2343 | 283 |
|
||||
| isTrustedCodehash | 993 | 1508 | 993 | 2993 | 198 |
|
||||
| rewardIndex | 821 | 846 | 821 | 2821 | 77 |
|
||||
| setTrustedCodehash | 52872 | 52872 | 52872 | 52872 | 51 |
|
||||
| totalMP | 756 | 756 | 756 | 756 | 80 |
|
||||
| totalMaxMP | 800 | 800 | 800 | 800 | 80 |
|
||||
| totalStaked | 756 | 756 | 756 | 756 | 81 |
|
||||
| updateAccountMP | 41712 | 43950 | 44214 | 44214 | 19 |
|
||||
| updateGlobalState | 37054 | 67298 | 56518 | 89469 | 28 |
|
||||
| upgradeToAndCall | 29767 | 35525 | 37445 | 37445 | 4 |
|
||||
| upgradeToAndCall | 29767 | 35900 | 37445 | 37445 | 5 |
|
||||
|
||||
|
||||
| src/StakeVault.sol:StakeVault contract | | | | | |
|
||||
@@ -80,9 +80,9 @@
|
||||
| Function Name | min | avg | median | max | # calls |
|
||||
| STAKING_TOKEN | 216 | 216 | 216 | 216 | 1 |
|
||||
| emergencyExit | 36353 | 48857 | 48091 | 65191 | 7 |
|
||||
| leave | 33507 | 53144 | 33507 | 92418 | 3 |
|
||||
| leave | 33507 | 132602 | 62962 | 370978 | 4 |
|
||||
| lock | 33245 | 60236 | 48577 | 110544 | 4 |
|
||||
| stake | 33454 | 242405 | 250826 | 271353 | 61 |
|
||||
| stake | 33454 | 242541 | 250826 | 271353 | 62 |
|
||||
| trustStakeManager | 28997 | 28997 | 28997 | 28997 | 1 |
|
||||
| unstake | 33260 | 117303 | 113502 | 156376 | 14 |
|
||||
| withdraw | 42227 | 42227 | 42227 | 42227 | 1 |
|
||||
@@ -161,12 +161,21 @@
|
||||
| Deployment Cost | Deployment Size | | | | |
|
||||
| 625454 | 3260 | | | | |
|
||||
| Function Name | min | avg | median | max | # calls |
|
||||
| approve | 46330 | 46339 | 46342 | 46342 | 247 |
|
||||
| balanceOf | 558 | 1387 | 558 | 2558 | 347 |
|
||||
| mint | 51279 | 58819 | 51279 | 68379 | 263 |
|
||||
| approve | 46330 | 46339 | 46342 | 46342 | 252 |
|
||||
| balanceOf | 558 | 1395 | 558 | 2558 | 351 |
|
||||
| mint | 51279 | 58806 | 51279 | 68379 | 268 |
|
||||
| transfer | 34384 | 48853 | 51484 | 51484 | 13 |
|
||||
|
||||
|
||||
| test/mocks/StackOverflowStakeManager.sol:StackOverflowStakeManager contract | | | | | |
|
||||
|-----------------------------------------------------------------------------|-----------------|--------|--------|--------|---------|
|
||||
| Deployment Cost | Deployment Size | | | | |
|
||||
| 1033443 | 4615 | | | | |
|
||||
| Function Name | min | avg | median | max | # calls |
|
||||
| leave | 391 | 161316 | 161316 | 322322 | 334 |
|
||||
| proxiableUUID | 319 | 319 | 319 | 319 | 1 |
|
||||
|
||||
|
||||
| test/mocks/XPProviderMock.sol:XPProviderMock contract | | | | | |
|
||||
|-------------------------------------------------------|-----------------|-------|--------|-------|---------|
|
||||
| Deployment Cost | Deployment Size | | | | |
|
||||
|
||||
@@ -13,6 +13,7 @@ LeaveTest:test_TrustNewStakeManager() (gas: 2642695)
|
||||
LockTest:test_LockFailsWithInvalidPeriod() (gas: 306175)
|
||||
LockTest:test_LockFailsWithNoStake() (gas: 61440)
|
||||
LockTest:test_LockWithoutPriorLock() (gas: 403273)
|
||||
MaliciousUpgradeTest:test_UpgradeStackOverflowStakeManager() (gas: 1761895)
|
||||
NFTMetadataGeneratorSVGTest:testGenerateMetadata() (gas: 85934)
|
||||
NFTMetadataGeneratorSVGTest:testSetImageStrings() (gas: 58332)
|
||||
NFTMetadataGeneratorSVGTest:testSetImageStringsRevert() (gas: 35804)
|
||||
|
||||
@@ -10,6 +10,7 @@ import { StakeVault } from "../src/StakeVault.sol";
|
||||
import { IStakeManagerProxy } from "../src/interfaces/IStakeManagerProxy.sol";
|
||||
import { StakeManagerProxy } from "../src/StakeManagerProxy.sol";
|
||||
import { MockToken } from "./mocks/MockToken.sol";
|
||||
import { StackOverflowStakeManager } from "./mocks/StackOverflowStakeManager.sol";
|
||||
|
||||
contract RewardsStreamerMPTest is Test {
|
||||
MockToken rewardToken;
|
||||
@@ -1948,3 +1949,43 @@ contract LeaveTest is RewardsStreamerMPTest {
|
||||
_leave(alice);
|
||||
}
|
||||
}
|
||||
|
||||
contract MaliciousUpgradeTest is RewardsStreamerMPTest {
|
||||
function _upgradeStakeManager() internal {
|
||||
address newImpl = address(new RewardsStreamerMP());
|
||||
bytes memory initializeData;
|
||||
UUPSUpgradeable(streamer).upgradeToAndCall(newImpl, initializeData);
|
||||
}
|
||||
|
||||
function setUp() public override {
|
||||
super.setUp();
|
||||
}
|
||||
|
||||
function test_UpgradeStackOverflowStakeManager() public {
|
||||
uint256 aliceInitialBalance = stakingToken.balanceOf(alice);
|
||||
|
||||
// first change the existing manager's state
|
||||
_stake(alice, 100e18, 0);
|
||||
checkStreamer(
|
||||
CheckStreamerParams({
|
||||
totalStaked: 100e18,
|
||||
totalMP: 100e18,
|
||||
totalMaxMP: 500e18,
|
||||
stakingBalance: 100e18,
|
||||
rewardBalance: 0,
|
||||
rewardIndex: 0,
|
||||
accountedRewards: 0
|
||||
})
|
||||
);
|
||||
|
||||
// upgrade the manager to a malicious one
|
||||
address newImpl = address(new StackOverflowStakeManager());
|
||||
bytes memory initializeData;
|
||||
UUPSUpgradeable(streamer).upgradeToAndCall(newImpl, initializeData);
|
||||
|
||||
// alice leaves system and is able to get funds out, despite malicious manager
|
||||
_leave(alice);
|
||||
|
||||
assertEq(stakingToken.balanceOf(alice), aliceInitialBalance, "Alice should get her tokens back");
|
||||
}
|
||||
}
|
||||
|
||||
69
test/mocks/StackOverflowStakeManager.sol
Normal file
69
test/mocks/StackOverflowStakeManager.sol
Normal file
@@ -0,0 +1,69 @@
|
||||
// SPDX-License-Identifier: MIT
|
||||
pragma solidity ^0.8.26;
|
||||
|
||||
import { IStakeManager } from "./../../src/interfaces/IStakeManager.sol";
|
||||
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
|
||||
import { TrustedCodehashAccess } from "./../../src/TrustedCodehashAccess.sol";
|
||||
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
|
||||
import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
|
||||
|
||||
contract StackOverflowStakeManager is
|
||||
UUPSUpgradeable,
|
||||
IStakeManager,
|
||||
TrustedCodehashAccess,
|
||||
ReentrancyGuardUpgradeable
|
||||
{
|
||||
IERC20 public STAKING_TOKEN;
|
||||
IERC20 public REWARD_TOKEN;
|
||||
|
||||
uint256 public constant SCALE_FACTOR = 1e18;
|
||||
uint256 public constant MP_RATE_PER_YEAR = 1e18;
|
||||
|
||||
uint256 public constant MIN_LOCKUP_PERIOD = 90 days;
|
||||
uint256 public constant MAX_LOCKUP_PERIOD = 4 * 365 days;
|
||||
uint256 public constant MAX_MULTIPLIER = 4;
|
||||
|
||||
uint256 public totalStaked;
|
||||
uint256 public totalMP;
|
||||
uint256 public totalMaxMP;
|
||||
uint256 public rewardIndex;
|
||||
uint256 public accountedRewards;
|
||||
uint256 public lastMPUpdatedTime;
|
||||
bool public emergencyModeEnabled;
|
||||
|
||||
struct Account {
|
||||
uint256 stakedBalance;
|
||||
uint256 accountRewardIndex;
|
||||
uint256 accountMP;
|
||||
uint256 maxMP;
|
||||
uint256 lastMPUpdateTime;
|
||||
uint256 lockUntil;
|
||||
}
|
||||
|
||||
mapping(address account => Account data) public accounts;
|
||||
|
||||
function getStakedBalance(address _vault) external view override returns (uint256 _balance) {
|
||||
// implementation
|
||||
}
|
||||
function lock(uint256 _seconds) external override {
|
||||
// implementation
|
||||
}
|
||||
function stake(uint256 _amount, uint256 _seconds) external override {
|
||||
// implementation
|
||||
}
|
||||
function unstake(uint256 _amount) external override {
|
||||
// implementation
|
||||
}
|
||||
|
||||
function leave() external override {
|
||||
this.leave();
|
||||
}
|
||||
|
||||
function _authorizeUpgrade(address) internal view override {
|
||||
_checkOwner();
|
||||
}
|
||||
|
||||
function getAccount(address _account) external view returns (Account memory) {
|
||||
return accounts[_account];
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user