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

Minting shares to the contract's own address during fee distribution increases its balance unexpectedly.

lin0x9a7a

Summary

Fee distribution bug artificially inflates the contract's balance by minting new shares to its own address.

  1. When updateStrategyRewards is called, it invokes the internal _updateStrategyRewards function.

  2. Inside _updateStrategyRewards, the function calculates the total rewards and fees earned by the strategies.

  3. If there are any fees to be distributed (totalFeeAmounts > 0), the function mints new shares to the contract's own address using the following code: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L577-L578

uint256 sharesToMint = (totalFeeAmounts * totalShares) / (totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint);

  1. The minting of new shares to the contract's own address increases the contract's balance.

  2. After minting the shares, the function proceeds to distribute the fees to the fee receivers using the transferAndCallFrom function.

In the _updateStrategyRewards function of the StakingPool contract, specifically in the following block: https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L576-L579

if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint);
// ...
}

This code block is responsible for minting new shares to the contract's own address, which leads to the increase in the contract's balance.

Vulnerability Details

When updateStrategyRewards is called, it invokes the internal _updateStrategyRewards function. Inside this function, if there are any fees to be distributed (totalFeeAmounts > 0), the contract mints new shares to its own address using the _mintShares function. This minting of shares artificially inflates the contract's balance without any actual deposits being made. https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L522-L600

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
// ...
​
// distribute fees to receivers if there are any
if (totalFeeAmounts > 0) {
uint256 sharesToMint = (totalFeeAmounts * totalShares) /
(totalStaked - totalFeeAmounts);
_mintShares(address(this), sharesToMint); // <--- Vuln: Minting shares to the contract's own address
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
transferAndCallFrom(
address(this), // <--- Vuln: Transferring from the contract's own address
receivers[i][j],
balanceOf(address(this)), // <--- Vuln: Using the inflated balance of the contract
"0x"
);
} else {
transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x"); // <--- Vuln: Transferring from the contract's own address
feesPaidCount++;
}
}
}
}
// ...
}

Impact

The fee distribution bug in the StakingPool contract has the following impact:

  • It artificially inflates the contract's balance without any actual deposits being made.

  • It leads to incorrect accounting and discrepancies in the expected balances of users and the contract itself.

  • It can potentially mislead users and auditors about the true state of the contract's funds.

Tools Used

Vs Code

Recommendations

diff --git a/contracts/core/StakingPool.sol b/contracts/core/StakingPool.sol
index ...
--- a/contracts/core/StakingPool.sol
+++ b/contracts/core/StakingPool.sol
@@ -...
// distribute fees to receivers if there are any
if (totalFeeAmounts > 0) {
- uint256 sharesToMint = (totalFeeAmounts * totalShares) /
- (totalStaked - totalFeeAmounts);
- _mintShares(address(this), sharesToMint);
-
uint256 feesPaidCount;
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers[i].length; j++) {
if (feesPaidCount == totalFeeCount - 1) {
- transferAndCallFrom(
- address(this),
+ token.safeTransfer(
receivers[i][j],
- balanceOf(address(this)),
- "0x"
+ token.balanceOf(address(this))
);
} else {
- transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
+ token.safeTransfer(receivers[i][j], feeAmounts[i][j]);
feesPaidCount++;
}
}
}
}

The proposed changes remove the minting of shares to the contract's own address during fee distribution. Instead, the fees are directly transferred to the fee receivers using the token.safeTransfer function.

Updates

Lead Judging Commences

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

Support

FAQs

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