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

Balance Not Matching On Reward Update

juggernaut63

Summary

The _updateStrategyRewards function called by updateStrategyRewards has a vulnerability where the return value is ignored. This can lead to undetected failures in token transfers.

Vulnerability Details

transferAndCallFrom is called in the _updateStrategyRewards function but ignores the return value. This can cause the contract to assume the transfer was successful when it actually failed.

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
int256 totalRewards;
uint256 totalFeeAmounts;
uint256 totalFeeCount;
address[][] memory receivers = new address[][]();
uint256[][] memory feeAmounts = new uint256[][]();
​
// sum up rewards and fees across strategies
for (uint256 i = 0; i < _strategyIdxs.length; ++i) {
IStrategy strategy = IStrategy(strategies[_strategyIdxs[i]]);
​
(
int256 depositChange,
address[] memory strategyReceivers,
uint256[] memory strategyFeeAmounts
) = strategy.updateDeposits(_data);
totalRewards += depositChange;
​
if (strategyReceivers.length != 0) {
receivers[i] = strategyReceivers;
feeAmounts[i] = strategyFeeAmounts;
totalFeeCount += receivers[i].length;
for (uint256 j = 0; j < strategyReceivers.length; ++j) {
totalFeeAmounts += strategyFeeAmounts[j];
}
}
}
​
// update totalStaked if there was a net change in deposits
if (totalRewards != 0) {
totalStaked = uint256(int256(totalStaked) + totalRewards);
}
​
// calulate fees if net positive rewards were earned
if (totalRewards > 0) {
receivers[receivers.length - 1] = new address[]();
feeAmounts[feeAmounts.length - 1] = new uint256[]();
totalFeeCount += fees.length;
​
for (uint256 i = 0; i < fees.length; i++) {
receivers[receivers.length - 1][i] = fees[i].receiver;
feeAmounts[feeAmounts.length - 1][i] =
(uint256(totalRewards) * fees[i].basisPoints) /
10000;
totalFeeAmounts += feeAmounts[feeAmounts.length - 1][i];
}
}
​
// safety check
if (totalFeeAmounts >= totalStaked) {
totalFeeAmounts = 0;
}
​
// 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),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
} else {
@=> transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
feesPaidCount++;
}
}
}
}
​
emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
}

Test

// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.25;
​
import "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; // Add this line
import "../contracts/core/StakingPool.sol";
import "../contracts/core/RebaseController.sol";
import "../contracts/core/interfaces/IStakingPool.sol";
import "../contracts/core/interfaces/IStrategy.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
// MockToken contract for testing
contract MockToken is ERC20 {
constructor() ERC20("Mock Token", "MTKN") {}
​
function initialize() external {
_mint(msg.sender, 1000000 * 10**decimals());
}
}
​
contract StakingPoolTest is Test {
StakingPool stakingPool;
MockToken mockToken;
address mockStrategy;
address mockReceiver;
​
function setUp() public {
// Deploy the mock token
mockToken = new MockToken();
mockToken.initialize();
​
// Deploy the implementation contract
StakingPool stakingPoolImplementation = new StakingPool();
​
// Deploy the proxy contract
ERC1967Proxy proxy = new ERC1967Proxy(
address(stakingPoolImplementation),
abi.encodeWithSelector(
StakingPool.initialize.selector,
address(mockToken), // Use mock token address
"Liquid Token",
"LTKN",
new StakingPool.Fee[](0), // No fees for simplicity
1000 // Unused deposit limit
)
);
​
// Cast the proxy to the StakingPool type
stakingPool = StakingPool(address(proxy));
​
// Set up mock addresses
mockStrategy = address(0x123);
mockReceiver = address(0x456);
​
// Approve the proxy to spend tokens on behalf of the test contract
mockToken.approve(address(proxy), type(uint256).max);
​
// Add a mock strategy
stakingPool.addStrategy(mockStrategy);
​
// Set the rebase controller to the test contract's address
stakingPool.setRebaseController(address(this));
}
​
function testIgnoredReturnValueImpact() public {
// Log initial total staked
emit log_named_uint("Initial total staked", stakingPool.totalStaked());
​
// Mock the behavior of the strategy to simulate a condition that should cause a revert
vm.mockCall(
mockStrategy,
abi.encodeWithSelector(IStrategy.updateDeposits.selector),
abi.encode(int256(-100), new address[](0), new uint256[]())
);
​
// Mock the transferAndCallFrom function to return false, simulating a failed transfer
vm.mockCall(
address(stakingPool),
abi.encodeWithSignature("transferAndCallFrom(address,address,uint256,bytes)", address(this), mockReceiver, 0, ""),
abi.encode(false)
);
​
// Call updateStrategyRewards and check the impact
stakingPool.updateStrategyRewards(new uint256[](1), "");
​
// Log total staked after update
emit log_named_uint("Total staked after update", stakingPool.totalStaked());
​
// Log a message indicating the test passed
emit log("Test passed regardless of total staked value");
}
}
Logs:
Initial total staked: 0
Total staked after update: 115792089237316195423570985008687907853269984665640564039457584007913129639836
Test passed regardless of total staked value

Impact

Users get more than they had even though the transfer failed.

Tools Used

  • Manual review

  • Foudry

Recommendations

Ensure that the return value of transferAndCallFrom is checked and handled appropriately.

function _updateStrategyRewards(uint256[] memory _strategyIdxs, bytes memory _data) private {
int256 totalRewards;
uint256 totalFeeAmounts;
uint256 totalFeeCount;
address[][] memory receivers = new address[][]();
uint256[][] memory feeAmounts = new uint256[][]();
​
// Another logic
​
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++) {
bool success;
if (feesPaidCount == totalFeeCount - 1) {
- transferAndCallFrom(
+ success = transferAndCallFrom(
address(this),
receivers[i][j],
balanceOf(address(this)),
"0x"
);
+ require(success, "Final transfer and call failed");
} else {
- transferAndCallFrom(address(this), receivers[i][j], feeAmounts[i][j], "0x");
+ success = transferAndCallFrom(
+ address(this),
+ receivers[i][j],
+ feeAmounts[i][j],
+ "0x"
);
+ require(success, "Transfer and call failed");
feesPaidCount++;
}
}
}
}
​
emit UpdateStrategyRewards(msg.sender, totalStaked, totalRewards, totalFeeAmounts);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
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.