Submission Details
Severity:
Staker loses pending rewards when they stakes more shares
Summary
Staker loses pending rewards when they stakes more shares.
Vulnerability Details
When a user calls stake() to stake a given amount of index tokens in the contract, protocol will try to accumulate the staker's pending reward by calling accumulateActor() defined in Distribution.
// accumulate the actor's pending reward before staking
wethRewardDistribution.accumulateActor(actorId);
accumulateActor() updates the staker's last value per share and returns the staker's pending rewards value since the their last staking event.
Distribution::accumulateActor():
function accumulateActor(Data storage self, bytes32 actorId) internal returns (SD59x18 valueChange) {
Actor storage actor = self.actor[actorId];
return _updateLastValuePerShare(self, actor, ud60x18(actor.shares));
}
However, as we can see in stake(), the returned pending rewards value is simply ignored, no rewards will be distributed to the staker nor the staking pending rewards are store for future claiming.
Please run the coded POC to verify:
// SPDX-License-Identifier: MIT
import { Test, console } from "forge-std/Test.sol";
import "./Base.t.sol";
import { Errors } from "@zaros/utils/Errors.sol";
import { Vault } from "@zaros/market-making/leaves/Vault.sol";
import { Collateral } from "@zaros/market-making/leaves/Collateral.sol";
contract AuditTest is Base_Test {
address owner = makeAddr("Owner");
address feeRecipient = makeAddr("FeeRecipient");
function setUp() override public {
// Perps Engine Set Up
bool isTestnet = false;
address[] memory branches = deployPerpsEngineBranches(isTestnet);
bytes4[][] memory branchesSelectors = getPerpsEngineBranchesSelectors(isTestnet);
RootProxy.BranchUpgrade[] memory branchUpgrades =
getBranchUpgrades(branches, branchesSelectors, RootProxy.BranchUpgradeAction.Add);
address[] memory initializables = getInitializables(branches);
bytes[] memory initializePayloads = getInitializePayloads(owner);
branchUpgrades = deployPerpsEngineHarnesses(branchUpgrades);
RootProxy.InitParams memory initParams = RootProxy.InitParams({
initBranches: branchUpgrades,
initializables: initializables,
initializePayloads: initializePayloads
});
perpsEngine = IPerpsEngine(address(new MockEngine(initParams)));
// Market Making Engine Set Up
address[] memory mmBranches = deployMarketMakingEngineBranches();
bytes4[][] memory mmBranchesSelectors = getMarketMakerBranchesSelectors();
RootProxy.BranchUpgrade[] memory mmBranchUpgrades =
getBranchUpgrades(mmBranches, mmBranchesSelectors, RootProxy.BranchUpgradeAction.Add);
RootProxy.BranchUpgrade[] memory mmbranchUpgrades = new RootProxy.BranchUpgrade[](mmBranchUpgrades.length);
for (uint256 i; i < mmbranchUpgrades.length; i++) {
mmbranchUpgrades[i] = mmBranchUpgrades[i];
}
initializables = getInitializables(mmBranches);
initializePayloads = getInitializePayloads(owner);
RootProxy.InitParams memory mmEngineInitParams = RootProxy.InitParams({
initBranches: mmbranchUpgrades,
initializables: initializables,
initializePayloads: initializePayloads
});
marketMakingEngine = IMarketMakingEngine(address(new MarketMakingEngine(mmEngineInitParams)));
vm.startPrank(owner);
// Configure Collaterals
uint256[2] memory marginCollateralIdsRange;
marginCollateralIdsRange[0] = INITIAL_MARGIN_COLLATERAL_ID;
marginCollateralIdsRange[1] = FINAL_MARGIN_COLLATERAL_ID;
mockSequencerUptimeFeed = address(new MockSequencerUptimeFeed(0));
configureMarginCollaterals(IPerpsEngine(perpsEngine), marginCollateralIdsRange, true, mockSequencerUptimeFeed, owner);
usdc = MockERC20(marginCollaterals[USDC_MARGIN_COLLATERAL_ID].marginCollateralAddress);
usdToken = MockUsdToken(marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].marginCollateralAddress);
weEth = MockERC20(marginCollaterals[WEETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wstEth = MockERC20(marginCollaterals[WSTETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wEth = MockERC20(marginCollaterals[WETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wBtc = MockERC20(marginCollaterals[WBTC_MARGIN_COLLATERAL_ID].marginCollateralAddress);
marketMakingEngine.configureCollateral(
address(usdToken),
marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].tokenDecimals
);
marketMakingEngine.configureCollateral(
address(wEth),
marginCollaterals[WETH_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[WETH_MARGIN_COLLATERAL_ID].tokenDecimals
);
// Set the wETH address
marketMakingEngine.setWeth(address(wEth));
// Configure Engine
marketMakingEngine.configureEngine(address(perpsEngine), address(usdToken), true);
// Configure Fee Recipient
marketMakingEngine.configureVaultDepositAndRedeemFeeRecipient(feeRecipient);
// Create Zlp Vaults
uint256[2] memory vaultsIdsRange;
vaultsIdsRange[0] = INITIAL_VAULT_ID;
vaultsIdsRange[1] = FINAL_VAULT_ID;
setupVaultsConfig();
createZlpVaults(address(marketMakingEngine), owner, vaultsIdsRange);
// Setup Perp Markets
bool isTest = true;
setupPerpMarketsCreditConfig(isTest, address(perpsEngine), address(usdToken));
vm.stopPrank();
}
function testAudit_UserLosesRewardsWhenStakesMore() public {
vm.startPrank(owner);
// Create WETH Vault
uint128 vaultId = WETH_CORE_VAULT_ID;
address indexToken = address(zlpVaults[vaultsConfig[vaultId].asset][vaultsConfig[vaultId].vaultType]);
ZlpVault zlpVault = ZlpVault(indexToken);
Collateral.Data memory collateral = Collateral.Data({
creditRatio: vaultsConfig[vaultId].creditRatio,
priceAdapter: vaultsConfig[vaultId].priceAdapter,
asset: vaultsConfig[vaultId].asset,
isEnabled: vaultsConfig[vaultId].isEnabled,
decimals: vaultsConfig[vaultId].decimals
});
Vault.CreateParams memory vaultCreatParams = Vault.CreateParams({
depositFee: vaultsConfig[vaultId].depositFee,
redeemFee: vaultsConfig[vaultId].redeemFee,
vaultId: vaultsConfig[vaultId].vaultId,
depositCap: vaultsConfig[vaultId].depositCap,
withdrawalDelay: vaultsConfig[vaultId].withdrawalDelay,
indexToken: indexToken,
engine: address(perpsEngine),
collateral: collateral
});
marketMakingEngine.createVault(vaultCreatParams);
Vault.UpdateParams memory vaultUpdateParams = Vault.UpdateParams({
vaultId: vaultId,
depositCap: vaultCreatParams.depositCap,
withdrawalDelay: vaultCreatParams.withdrawalDelay,
isLive: true,
lockedCreditRatio: 0
});
marketMakingEngine.updateVaultConfiguration(vaultUpdateParams);
// Configure Market
uint128 marketCreditConfigId = ETH_PERP_MARKET_CREDIT_CONFIG_ID;
uint128 marketId = perpMarketsCreditConfig[marketCreditConfigId].marketId;
marketMakingEngine.configureMarket(
address(perpsEngine),
perpMarketsCreditConfig[marketCreditConfigId].marketId,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageStartThreshold,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageEndThreshold,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageExpoentZ
);
// Connect Vault and Market
uint256[] memory marketIds = new uint256[](1);
marketIds[0] = perpMarketsCreditConfig[marketCreditConfigId].marketId;
uint256[] memory vaultIds = new uint256[](1);
vaultIds[0] = vaultsConfig[vaultId].vaultId;
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
marketMakingEngine.updateVaultCreditCapacity(vaultsConfig[vaultId].vaultId);
vm.stopPrank();
address alice = makeAddr("Alice");
uint256 depositAmount = 2e18;
wEth.mint(alice, depositAmount);
vm.startPrank(alice);
// Alice deposits into vault
wEth.approve(address(marketMakingEngine), depositAmount);
marketMakingEngine.deposit(vaultsConfig[vaultId].vaultId, uint128(depositAmount), 0, "", false);
// Alice stakes half of the minted shares
uint256 shares = zlpVault.balanceOf(alice);
zlpVault.approve(address(marketMakingEngine), shares / 2);
marketMakingEngine.stake(vaultId, uint128(shares / 2));
vm.stopPrank();
// Receive Market Fee
wEth.mint(address(perpsEngine), 1e18);
vm.startPrank(address(perpsEngine));
wEth.approve(address(marketMakingEngine), 1e18);
marketMakingEngine.receiveMarketFee(marketId, address(wEth), 1e18);
vm.stopPrank();
// Alice stakes the other half of the minted shares
vm.startPrank(alice);
zlpVault.approve(address(marketMakingEngine), shares / 2);
marketMakingEngine.stake(vaultId, uint128(shares / 2));
vm.stopPrank();
// Alice loses rewards
vm.prank(alice);
vm.expectRevert(Errors.NoFeesToClaim.selector);
marketMakingEngine.claimFees(vaultId);
}
}
Impact
User loses rewards.
Tools Used
Manual Review
Recommendations
When user stakes, it is recommended to store the user's pending rewards for future claming, or simply distribute the rewards to the user.
Updates
Lead Judging Commences
inallhonesty 4 months ago
Submission Judgement Published Assigned finding tags:
Inside VaultRouterBranch if you stake wait some time then stake again makes you lose the rewards.
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821 USDC / LOC
66,500
3,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.