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

Principal management vulnerability lead to incorrect reward calculations or unintended withdrawals in `LSTRewardsSplitter.sol`

safdie

Summary

The LSTRewardsSplitter contract has a vulnerability in how it manages principal deposits. Specifically, the contract allows negative rewards to be deducted directly from principalDeposits, which can cause confusion and potential issues with the contract's balance. If principalDeposits falls below its actual value, it may lead to incorrect reward calculations or unintended withdrawals.

Vulnerability Details

In the performUpkeep and splitRewards functions, the contract calculates the difference between the current token balance and the stored principalDeposits. This difference is considered "new rewards." If the calculated difference is negative (i.e., if there are fewer tokens than the principalDeposits), the contract reduces the value of principalDeposits by the negative difference. This reduction blurs the distinction between principal and rewards, potentially causing confusion and making it difficult to accurately track user deposits versus the rewards generated by the system.

A specific concern arises when negative rewards are encountered. Reducing the principalDeposits in this manner can make it unclear what portion of the contract’s balance is principal (the original deposit) and what portion is rewards (additional tokens). Over time, this can lead to inaccuracies in tracking the correct amount that should be withdrawn or distributed to users. Furthermore, if the principal becomes incorrect, users may withdraw more tokens than intended or lose some of their rightful deposits.

The following Hardhat test simulates the scenario where negative rewards lead to a decrease in principal:

describe("Principal Management Vulnerability", function () {
it("Should incorrectly reduce principalDeposits on negative rewards", async function () {
const [owner, user] = await ethers.getSigners();
const lst = await ethers.getContractFactory("ERC677TokenMock"); // Mock of the LST token
const lstRewardsSplitter = await ethers.getContractFactory("LSTRewardsSplitter");
​
// Deploy a mock LST token and the LSTRewardsSplitter contract
const lstInstance = await lst.deploy("Liquid Staking Token", "LST", 1000000);
const splitterInstance = await lstRewardsSplitter.deploy(lstInstance.address, [], owner.address);
​
// Owner deposits 1000 tokens
await lstInstance.transfer(owner.address, 1000);
await lstInstance.approve(splitterInstance.address, 1000);
await splitterInstance.deposit(1000);
expect(await splitterInstance.principalDeposits()).to.equal(1000);
​
// Manually reduce the balance of the contract to simulate negative rewards
await lstInstance.transfer(user.address, 500); // Simulating external reduction of contract balance
expect(await lstInstance.balanceOf(splitterInstance.address)).to.equal(500);
​
// Call the splitRewards function, which should adjust principalDeposits
await splitterInstance.splitRewards();
expect(await splitterInstance.principalDeposits()).to.equal(500); // This is incorrect and could lead to withdrawal issues
​
// Test withdrawing the remaining balance
await splitterInstance.withdraw(500, user.address);
expect(await lstInstance.balanceOf(user.address)).to.equal(500);
expect(await splitterInstance.principalDeposits()).to.equal(0); // PrincipalDeposits becomes zero, but incorrect management of deposits
});
});

Impact

The vulnerability in principal management can lead to incorrect tracking of deposits and rewards, which may allow:

  • Users to withdraw more tokens than they have deposited, causing financial loss to the contract.

  • The contract balance to be misrepresented, leading to an imbalance in rewards distribution.

  • Over time, as more rewards are distributed, the principal deposit amount could become severely corrupted, leading to significant losses or unintended withdrawals.

Tools Used

Manual review.

Recommendations

To mitigate this issue, the contract should clearly separate the management of principal deposits and rewards:

  1. Ensure that principal deposits are always tracked independently from rewards, and do not allow negative rewards to modify principalDeposits.

  2. If negative rewards are detected, do not allow the principal to be adjusted beyond the actual deposit amount.

  3. Add checks to ensure that principalDeposits always accurately reflects the original deposit amounts and that any reduction is handled explicitly.

  4. If the balance is reduced externally (outside of user withdrawals), the contract should handle this event as an error instead of modifying the principal.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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