Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Logic flaw in `updateDeposits` function leading to inaccurate accounting in `OperatorVault.sol`

safdie

Summary

The updateDeposits function in the OperatorVault contract has a potential logic flaw that can lead to incorrect reward calculations and unintended behaviors.

Vulnerability Details

The updateDeposits function is responsible for updating the deposit and reward accounting for the vault. The logic used to calculate the newly earned rewards and adjust tracked total deposits may be flawed, especially if the contract does not account for changes in deposits accurately. This can lead to scenarios where rewards are either over-claimed or under-claimed, potentially affecting the financial integrity of the contract.

The updateDeposits function recalculates deposits and rewards based on the difference between total deposits and trackedTotalDeposits. However, the condition if (depositChange > 0) only allows positive changes to affect rewards. If there are negative changes, they might not be accounted for properly.

function updateDeposits(
uint256 _minRewards,
address _rewardsReceiver
) external onlyVaultController returns (uint256, uint256, uint256) {
uint256 principal = getPrincipalDeposits();
uint256 rewards = getRewards();
uint256 totalDeposits = principal + rewards;
int256 depositChange = int256(totalDeposits) - int256(uint256(trackedTotalDeposits));
​
uint256 opRewards;
if (depositChange > 0) {
opRewards =
(uint256(depositChange) *
IOperatorVCS(vaultController).operatorRewardPercentage()) /
10000;
unclaimedRewards += SafeCast.toUint128(opRewards);
trackedTotalDeposits = SafeCast.toUint128(totalDeposits);
}
​
if (_minRewards != 0 && rewards >= _minRewards) {
rewardsController.claimReward();
trackedTotalDeposits -= SafeCast.toUint128(rewards);
totalDeposits -= rewards;
token.safeTransfer(_rewardsReceiver, rewards);
}
​
return (totalDeposits, principal, opRewards);
}
​

Issue here is that:

  1. The way the depositChange is calculated does not effectively handle edge cases where deposits might decrease, leading to incorrect calculations for opRewards.

  2. If rewards are not accurately accounted for when deposits decrease, the operator may miss out on rewards that they are entitled to.

  3. The management of trackedTotalDeposits does not effectively account for changes in rewards, leading to potential discrepancies in the rewards distribution logic.

PoC:

  1. Scenario: An operator deposits a certain amount, and the rewards are updated. If the deposits later decrease (for example, due to an improper withdrawal or mistake), the logic may not adjust correctly, causing rewards to be miscalculated.

  2. Realistic PoC:

  • Deploy the OperatorVault contract.

  • Deposit a certain amount and accumulate rewards.

  • Simulate a decrease in deposits and observe how the reward calculation fails.

// Step 1: Deploy contract and set up necessary parameters
OperatorVault operatorVault = new OperatorVault();
​
// Step 2: Simulate initial deposit
await operatorVault.deposit(1000); // Initial deposit
await operatorVault.updateDeposits(0, rewardsReceiver.address); // Update rewards
​
// Step 3: Simulate a decrease in deposits
await operatorVault.withdraw(500); // Withdraw half
await operatorVault.updateDeposits(0, rewardsReceiver.address); // Update rewards
​
// Step 4: Check rewards
const rewardsAfterWithdrawal = await operatorVault.getPendingRewards();

Expected output:
The rewards calculation after the withdrawal should accurately reflect the change in deposits. If the logic flaw is present, it may result in an incorrect rewards amount.
Here’s a sample test script using Hardhat to demonstrate the potential logic flaw in the updateDeposits function:

const { expect } = require("chai");
const { ethers } = require("hardhat");
​
describe("OperatorVault - Logic Flaw in updateDeposits", function () {
let operatorVault, owner, rewardsReceiver, addr1;
​
beforeEach(async function () {
[owner, rewardsReceiver, addr1] = await ethers.getSigners();
​
const OperatorVault = await ethers.getContractFactory("OperatorVault");
operatorVault = await OperatorVault.deploy();
await operatorVault.initialize(
addr1.address, // token
owner.address, // vault controller
addr1.address, // staking controller
addr1.address, // rewards controller
addr1.address, // pfAlertsController
rewardsReceiver.address, // operator
rewardsReceiver.address // rewards receiver
);
});
​
it("Should correctly calculate rewards after deposits decrease", async function () {
// Initial deposit
await operatorVault.deposit(1000);
await operatorVault.updateDeposits(0, rewardsReceiver.address);
​
// Simulate rewards accumulation
await operatorVault.updateDeposits(0, rewardsReceiver.address);
​
// Withdraw part of the deposits
await operatorVault.withdraw(500);
const pendingRewardsBeforeUpdate = await operatorVault.getPendingRewards();
​
// Update deposits again
await operatorVault.updateDeposits(0, rewardsReceiver.address);
const pendingRewardsAfterUpdate = await operatorVault.getPendingRewards();
​
expect(pendingRewardsAfterUpdate).to.equal(pendingRewardsBeforeUpdate);
});
});

Impact

  1. If the logic flaw persists, operators may miss out on rewards, leading to financial losses.

  2. Users may lose trust in the system if they perceive that rewards are not managed correctly.

  3. Inefficient reward calculations may lead to increased gas costs and resource usage.

Tools Used

Manual review.

Recommendations

  1. Ensure that the logic accurately accounts for all changes in deposits, including reductions due to withdrawals.

  2. Introduce conditions that validate the integrity of deposits and rewards before proceeding with updates.

  3. Implement comprehensive unit tests to catch potential edge cases in reward calculations, and conduct regular code reviews to identify logical flaws.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.