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

Stakelink Liquid Staking Audit Report (by coinleft)

coinleft

Vulnerability Details

1.Reentrancy in OperatorVCS.updateDeposits(bytes)

Path: contracts/linkStaking/OperatorVCS.sol#159-223

Description:
token.safeTransfer(address(stakingPool), balance) in the OperatorVCS.sol, which transfers tokens to the stakingPool. Since this involves an interaction with an external contract (stakingPool), there is a potential risk of a reentrant call during the token transfer. The contract state variables (totalDeposits, totalPrincipalDeposits) are updated only after the external call is completed, making the contract vulnerable if stakingPool is malicious and attempts reentrancy.

Code:

function updateDeposits(
bytes calldata _data
)
external
override
onlyStakingPool
returns (int256 depositChange, address[] memory receivers, uint256[] memory amounts)
{
// ...
if (balance != 0) {
token.safeTransfer(address(stakingPool), balance);
newTotalDeposits -= balance;
}
​
totalDeposits = newTotalDeposits;
totalPrincipalDeposits = newTotalPrincipalDeposits;
}
​

2.Reentrancy in VaultDepositController._depositToVaults(uint256,uint256,uint256,uint64[]).

Path: contracts/linkStaking/base/VaultControllerStrategy.sol#172-292

Description:

The contract’s state variables group.totalDepositRoom and toDeposit are updated only after the call to vault.deposit(uint256), which makes the contract vulnerable if vault becomes malicious. A malicious vault could exploit this by causing multiple deposit failures, draining all funds and future deposits.

Code:

​
// ...
​
if (toDeposit > canDeposit) {
vault.deposit(canDeposit);
toDeposit -= canDeposit;
group.totalDepositRoom -= uint128(canDeposit);
} else {
vault.deposit(toDeposit);
group.totalDepositRoom -= uint128(toDeposit);
toDeposit = 0;
break;
}

3.Reentrancy in VaultDepositController.withdraw(uint256,bytes)

Path: contracts/linkStaking/base/VaultControllerStrategy.sol#111-163

Description: In the for loop, vault.withdraw(deposits) is called before updating unbondedRemaining, which impacts the subsequent state variable totalDeposits, making the contract vulnerable with reentrancy.

Code:

function withdraw(uint256 _amount, bytes calldata _data) external {
// ...
​
for (uint256 i = 0; i < vaultIds.length; ++i) {
// vault must be a member of the current unbonded group
if (vaultIds[i] % globalState.numVaultGroups != globalState.curUnbondedVaultGroup)
revert InvalidVaultIds();
​
group.withdrawalIndex = uint64(vaultIds[i]);
IVault vault = vaults[vaultIds[i]];
uint256 deposits = vault.getPrincipalDeposits();
​
if (deposits != 0 && vault.claimPeriodActive() && !vault.isRemoved()) {
if (toWithdraw > deposits) {
vault.withdraw(deposits);
unbondedRemaining -= deposits;
toWithdraw -= deposits;
} else if (deposits - toWithdraw > 0 && deposits - toWithdraw < minDeposits) {
// cannot leave a vault with less than minimum deposits
vault.withdraw(deposits);
unbondedRemaining -= deposits;
break;
} else {
vault.withdraw(toWithdraw);
unbondedRemaining -= toWithdraw;
break;
}
}
}
​
// ...
​
}

4.Reentrancy in VaultDepositController.withdraw(uint256,bytes)

Path: contracts/linkStaking/base/VaultControllerStrategy.sol#111-163

Description: token.safeTransfer(msg.sender, totalWithdrawn) called before update state variables totalUnbonded,totalPrincipalDeposits,group.totalDepositRoom,vaultGroups[], making the contract vulnerable with reentrancy.

Code:

function withdraw(uint256 _amount, bytes calldata _data) external {
// ...
​
uint256 totalWithdrawn = totalUnbonded - unbondedRemaining;
​
token.safeTransfer(msg.sender, totalWithdrawn);
​
totalDeposits -= totalWithdrawn;
totalPrincipalDeposits -= totalWithdrawn;
totalUnbonded = unbondedRemaining;
​
group.totalDepositRoom += uint128(totalWithdrawn);
vaultGroups[globalVaultState.curUnbondedVaultGroup] = group;
}

5.Reentrancy in VaultControllerStrategy.updateDeposits(bytes).

Path: contracts/linkStaking/base/VaultControllerStrategy.sol#525-557

Description: token.safeTransfer(address,uint256) called before update state variable totalDeposits, making the contract vulnerable with reentrancy.

Code:

function updateDeposits(bytes) {
// ...
​
uint256 balance = token.balanceOf(address(this));
if (balance != 0) {
token.safeTransfer(address(stakingPool), balance);
newTotalDeposits -= balance;
}
​
totalDeposits = newTotalDeposits;
}

Impact

A reentrancy attack could potentially lead to an inconsistent state in the contract, where accounting variables do not reflect the actual balance, which might be exploited for financial gain.

Recommendations

To mitigate reentrancy risks:

  • Use a reentrancy guard, such as ReentrancyGuard from OpenZeppelin, to protect the function.

  • Alternatively, move the state updates (totalDeposits and totalPrincipalDeposits) before the external call to safeTransfer, ensuring the contract is in a consistent state before making external interactions.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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