From 9a4bed131ce2740491d91bdb11d2bc69ad4c8860 Mon Sep 17 00:00:00 2001 From: kyriediculous Date: Fri, 15 Mar 2024 14:00:30 +0100 Subject: [PATCH] security: fix polygon vaults liveness loss if validator owner changes --- src/adapters/PolygonAdapter.sol | 70 +++++++++++++++++++++------ src/adapters/interfaces/IPolygon.sol | 4 ++ test/fixes/advisory-fix-1.sol | 46 ++++++++++++++++++ test/fork-tests/Polygon.mainnet.t.sol | 15 +++--- 4 files changed, 112 insertions(+), 23 deletions(-) create mode 100644 test/fixes/advisory-fix-1.sol diff --git a/src/adapters/PolygonAdapter.sol b/src/adapters/PolygonAdapter.sol index 1b96ba3..98cea73 100644 --- a/src/adapters/PolygonAdapter.sol +++ b/src/adapters/PolygonAdapter.sol @@ -18,7 +18,12 @@ import { ERC20 } from "solmate/tokens/ERC20.sol"; import { Adapter } from "core/adapters/Adapter.sol"; import { IERC165 } from "core/interfaces/IERC165.sol"; import { ITenderizer } from "core/tenderizer/ITenderizer.sol"; -import { IPolygonStakeManager, IPolygonValidatorShares, DelegatorUnbond } from "core/adapters/interfaces/IPolygon.sol"; +import { + IPolygonStakeManager, + IPolygonValidatorShares, + IPolygonStakingNFT, + DelegatorUnbond +} from "core/adapters/interfaces/IPolygon.sol"; // Matic exchange rate precision uint256 constant EXCHANGE_RATE_PRECISION = 100; // For Validator ID < 8 @@ -28,6 +33,13 @@ uint256 constant WITHDRAW_DELAY = 80; // 80 epochs, epoch length can vary on ave IPolygonStakeManager constant POLYGON_STAKEMANAGER = IPolygonStakeManager(address(0x5e3Ef299fDDf15eAa0432E6e66473ace8c13D908)); ERC20 constant POL = ERC20(address(0x7D1AfA7B718fb893dB30A3aBc0Cfc608AaCfeBB0)); +// Old validator owners +// If validators changed their "owner" we must have a special case to fetch the +// correct validator shares contract for their new owner. +// This fixes issue where tenderizers are currently tied to "owner" instead of "validatorId" +address constant BOUNTYBLOK_OLD = 0x055BD801cA712b4ddf67db8BC23FB6C8510D52b9; +address constant BOUNTYBLOK_NEW = 0x1BE946281214Afa0200725917B46EaeCb4b7dBE1; + // Polygon validators with a `validatorId` less than 8 are foundation validators // These are special case validators that don't have slashing enabled and still operate // On the old precision for the ValidatorShares contract. @@ -42,6 +54,21 @@ function getExchangePrecision(uint256 validatorId) pure returns (uint256) { contract PolygonAdapter is Adapter { using SafeTransferLib for ERC20; + struct Storage { + uint256 validatorId; + } + + uint256 private constant STORAGE = uint256(keccak256("xyz.tenderize.polygon.adapter.storage.location")) - 1; + + function _loadStorage() internal pure returns (Storage storage $) { + uint256 slot = STORAGE; + + // solhint-disable-next-line no-inline-assembly + assembly { + $.slot := slot + } + } + function supportsInterface(bytes4 interfaceId) external pure override returns (bool) { return interfaceId == type(Adapter).interfaceId || interfaceId == type(IERC165).interfaceId; } @@ -50,15 +77,15 @@ contract PolygonAdapter is Adapter { // Validator must have a validator shares contract // This will revert if the address does not own its StakeNFT // Which could lead to unexpected behaviour if used by external contracts - return address(_getValidatorSharesContract(_getValidatorId(validator))) != address(0); + return address(_getValidatorSharesContract(getValidatorId(validator))) != address(0); } function previewDeposit(address validator, uint256 assets) external view returns (uint256) { - uint256 validatorId = _getValidatorId(validator); + uint256 validatorId = getValidatorId(validator); uint256 delegatedAmount = IPolygonStakeManager(POLYGON_STAKEMANAGER).delegatedAmount(validatorId); IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); uint256 totalShares = validatorShares.totalSupply(); - uint256 prec = getExchangePrecision(_getValidatorId(validator)); + uint256 prec = getExchangePrecision(getValidatorId(validator)); uint256 fxRate_0 = prec * delegatedAmount / totalShares; uint256 sharesToMint = assets * prec / fxRate_0; uint256 amountToTransfer = sharesToMint * fxRate_0 / prec; @@ -70,7 +97,7 @@ contract PolygonAdapter is Adapter { // get validator for caller (Tenderizer through delegate call) address validator = _getValidatorAddress(); // get the validator shares contract for validator - uint256 validatorId = _getValidatorId(validator); + uint256 validatorId = getValidatorId(validator); IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); DelegatorUnbond memory unbond = validatorShares.unbonds_new(address(this), unlockID); @@ -85,7 +112,7 @@ contract PolygonAdapter is Adapter { // While we could use an historical average, it's better to just return the epoch number for now // consumers of this method can still convert it into a block or timestamp if they choose to DelegatorUnbond memory u = - _getValidatorSharesContract(_getValidatorId(_getValidatorAddress())).unbonds_new(address(this), unlockID); + _getValidatorSharesContract(getValidatorId(_getValidatorAddress())).unbonds_new(address(this), unlockID); return u.withdrawEpoch + WITHDRAW_DELAY; } @@ -101,7 +128,7 @@ contract PolygonAdapter is Adapter { // approve tokens POL.safeApprove(address(POLYGON_STAKEMANAGER), amount); - uint256 validatorId = _getValidatorId(validator); + uint256 validatorId = getValidatorId(validator); IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); // calculate minimum amount of voucher shares to mint @@ -115,7 +142,7 @@ contract PolygonAdapter is Adapter { } function unstake(address validator, uint256 amount) external override returns (uint256 unlockID) { - uint256 validatorId = _getValidatorId(validator); + uint256 validatorId = getValidatorId(validator); IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); uint256 precision = getExchangePrecision(validatorId); @@ -130,7 +157,7 @@ contract PolygonAdapter is Adapter { } function withdraw(address validator, uint256 unlockID) external override returns (uint256 amount) { - uint256 validatorId = _getValidatorId(validator); + uint256 validatorId = getValidatorId(validator); IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); DelegatorUnbond memory unbond = validatorShares.unbonds_new(address(this), unlockID); @@ -143,7 +170,16 @@ contract PolygonAdapter is Adapter { } function rebase(address validator, uint256 currentStake) external returns (uint256 newStake) { - uint256 validatorId = _getValidatorId(validator); + // Special case code to fix validators that have changed owner + if (validator == BOUNTYBLOK_OLD) { + validator = BOUNTYBLOK_NEW; + } + + Storage storage $ = _loadStorage(); + uint256 validatorId = getValidatorId(validator); + if ($.validatorId == 0) { + $.validatorId = validatorId; + } IPolygonValidatorShares validatorShares = _getValidatorSharesContract(validatorId); // This call will revert if there are no rewards @@ -162,13 +198,17 @@ contract PolygonAdapter is Adapter { } function _getValidatorAddress() internal view returns (address) { - return ITenderizer(address(this)).validator(); + Storage storage $ = _loadStorage(); + uint256 id = $.validatorId; + return id != 0 ? IPolygonStakingNFT(address(POLYGON_STAKEMANAGER)).ownerOf(id) : ITenderizer(address(this)).validator(); } -} -function _getValidatorId(address validator) view returns (uint256) { - // This will revert if validator is not valid - return POLYGON_STAKEMANAGER.getValidatorId(validator); + function getValidatorId(address validator) public view returns (uint256) { + // This will revert if validator is not valid + Storage storage $ = _loadStorage(); + uint256 id = $.validatorId; + return id != 0 ? id : POLYGON_STAKEMANAGER.getValidatorId(validator); + } } function _getValidatorSharesContract(uint256 validatorId) view returns (IPolygonValidatorShares) { diff --git a/src/adapters/interfaces/IPolygon.sol b/src/adapters/interfaces/IPolygon.sol index 4896fd5..f0a3d37 100644 --- a/src/adapters/interfaces/IPolygon.sol +++ b/src/adapters/interfaces/IPolygon.sol @@ -43,3 +43,7 @@ interface IPolygonValidatorShares { function totalSupply() external view returns (uint256); } + +interface IPolygonStakingNFT { + function ownerOf(uint256 tokenId) external view returns (address); +} diff --git a/test/fixes/advisory-fix-1.sol b/test/fixes/advisory-fix-1.sol new file mode 100644 index 0000000..340293f --- /dev/null +++ b/test/fixes/advisory-fix-1.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MIT +// +// _____ _ _ +// |_ _| | | (_) +// | | ___ _ __ __| | ___ _ __ _ _______ +// | |/ _ \ '_ \ / _` |/ _ \ '__| |_ / _ \ +// | | __/ | | | (_| | __/ | | |/ / __/ +// \_/\___|_| |_|\__,_|\___|_| |_/___\___| +// +// Copyright (c) Tenderize Labs Ltd + +pragma solidity >=0.8.19; + +import { Test } from "forge-std/Test.sol"; +import { VmSafe } from "forge-std/Vm.sol"; + +import { PolygonAdapter, POL } from "core/adapters/PolygonAdapter.sol"; +import { Tenderizer } from "core/tenderizer/ITenderizer.sol"; +import { Registry } from "core/registry/Registry.sol"; + +address constant VALIDATOR_OLD = 0x055BD801cA712b4ddf67db8BC23FB6C8510D52b9; +address constant VALIDATOR_NEW = 0x1BE946281214Afa0200725917B46EaeCb4b7dBE1; +address payable constant TENDERIZER = payable(0xa536981111f0C1e510150c544D8762Ae8e9bEbd3); +address constant REGISTRY = 0xa7cA8732Be369CaEaE8C230537Fc8EF82a3387EE; +address constant GOVERNOR = 0x5542b58080FEE48dBE6f38ec0135cE9011519d96; + +contract Polygon_Advisory_Fix_1 is Test { + function setUp() public { + vm.createSelectFork(vm.envString("MAINNET_RPC")); + } + + function test_rebase_fails() public { + Tenderizer tenderizer = Tenderizer(TENDERIZER); + vm.expectRevert(); + tenderizer.rebase(); + } + + function test_fixed_adapter() public { + address adapter = address(new PolygonAdapter()); + Registry registry = Registry(REGISTRY); + Tenderizer tenderizer = Tenderizer(TENDERIZER); + vm.prank(GOVERNOR); + registry.registerAdapter(address(POL), adapter); + tenderizer.rebase(); + } +} diff --git a/test/fork-tests/Polygon.mainnet.t.sol b/test/fork-tests/Polygon.mainnet.t.sol index 9027c21..1900443 100644 --- a/test/fork-tests/Polygon.mainnet.t.sol +++ b/test/fork-tests/Polygon.mainnet.t.sol @@ -21,8 +21,7 @@ import { POL, WITHDRAW_DELAY, EXCHANGE_RATE_PRECISION_HIGH, - _getValidatorSharesContract, - _getValidatorId + _getValidatorSharesContract } from "core/adapters/PolygonAdapter.sol"; import { IPolygonStakeManager, IPolygonValidatorShares } from "core/adapters/interfaces/IPolygon.sol"; import { Tenderizer, TenderizerEvents } from "core/tenderizer/Tenderizer.sol"; @@ -53,7 +52,7 @@ contract PolygonForkTest is Test, TenderizerEvents, ERC721Receiver { event NewTenderizer(address indexed asset, address indexed validator, address tenderizer); function setRewards(uint256 amount, uint256 initialRewardPerShare) internal returns (uint256 rewardPerShare) { - IPolygonValidatorShares valShares = _getValidatorSharesContract(_getValidatorId(VALIDATOR_1)); + IPolygonValidatorShares valShares = _getValidatorSharesContract(adapter.getValidatorId(VALIDATOR_1)); uint256 totalShares = valShares.totalSupply(); rewardPerShare = initialRewardPerShare + amount * REWARD_PRECISION / totalShares; // We have to update the `Validator.delegatorsRewards` for our validator @@ -106,9 +105,9 @@ contract PolygonForkTest is Test, TenderizerEvents, ERC721Receiver { function testFuzz_previewDeposit(uint256 amount) public { amount = bound(amount, 1, 10e28); - IPolygonValidatorShares valShares = _getValidatorSharesContract(_getValidatorId(VALIDATOR_1)); + IPolygonValidatorShares valShares = _getValidatorSharesContract(adapter.getValidatorId(VALIDATOR_1)); uint256 totalShares = valShares.totalSupply(); - uint256 delegatedAmount = POLYGON_STAKEMANAGER.delegatedAmount(_getValidatorId(VALIDATOR_1)); + uint256 delegatedAmount = POLYGON_STAKEMANAGER.delegatedAmount(adapter.getValidatorId(VALIDATOR_1)); uint256 preview = adapter.previewDeposit(VALIDATOR_1, amount); uint256 mintedPolShares = amount * EXCHANGE_RATE_PRECISION_HIGH / (delegatedAmount * EXCHANGE_RATE_PRECISION_HIGH / totalShares); @@ -126,9 +125,9 @@ contract PolygonForkTest is Test, TenderizerEvents, ERC721Receiver { Tenderizer tenderizer = Tenderizer(payable(fixture.factory.newTenderizer(address(POL), VALIDATOR_1))); - IPolygonValidatorShares valShares = _getValidatorSharesContract(_getValidatorId(VALIDATOR_1)); + IPolygonValidatorShares valShares = _getValidatorSharesContract(adapter.getValidatorId(VALIDATOR_1)); uint256 totalShares = valShares.totalSupply(); - uint256 delegatedAmount = POLYGON_STAKEMANAGER.delegatedAmount(_getValidatorId(VALIDATOR_1)); + uint256 delegatedAmount = POLYGON_STAKEMANAGER.delegatedAmount(adapter.getValidatorId(VALIDATOR_1)); uint256 preview = tenderizer.previewDeposit(amount); uint256 fxRateBefore = delegatedAmount * EXCHANGE_RATE_PRECISION_HIGH / totalShares; @@ -218,7 +217,7 @@ contract PolygonForkTest is Test, TenderizerEvents, ERC721Receiver { POL.approve(address(tenderizer), HOLDER_2_DEPOSIT); uint256 tTokenOut_2 = tenderizer.deposit(HOLDER_2, HOLDER_2_DEPOSIT); vm.stopPrank(); - IPolygonValidatorShares valShares = _getValidatorSharesContract(_getValidatorId(VALIDATOR_1)); + IPolygonValidatorShares valShares = _getValidatorSharesContract(adapter.getValidatorId(VALIDATOR_1)); uint256 tenderizerValShares = valShares.balanceOf(address(tenderizer)); uint256 initialRewardPerShare = IPolygonValidatorSharesTest(address(valShares)).initalRewardPerShare(address(tenderizer));