MRXSNOWDEN - Reentrancy Vulnerability in The _claim function

[sherlock-admin4]

MRXSNOWDEN

medium

Reentrancy Vulnerability in The _claim function

mrxsnowden medium

Reentrancy Vulnerability in The _claim function

Summary

The _claim function processes claims by iterating through the winners and prize indices arrays and calling the claimPrize function on the _vault contract. This external call to _vault.claimPrize can be potentially dangerous if the external contract is compromised or behaves maliciously, leading to a reentrancy attack. In a reentrancy attack, the external contract (or a malicious actor) can repeatedly call back into the original contract before the first invocation is completed, manipulating the contract state and potentially draining funds or causing unintended behavior.

Vulnerability Detail

If an attacker manages to exploit this vulnerability, they can repeatedly invoke the claimPrize function, reentering the _claim function multiple times before it completes its execution.

Impact

This can lead to incorrect state updates, unexpected contract behavior, and potentially significant financial loss. Given the nature of prize claims and the financial incentives involved, the impact of such an attack can be substantial.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-claimer/src/Claimer.sol#L90 https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-prize-pool/src/libraries/DrawAccumulatorLib.sol#L63

Tool used

Manual Review

Recommendation

Solution: Use the nonReentrant modifier from OpenZeppelin's ReentrancyGuard to prevent reentrancy attacks.

Implementation Steps:

Import the ReentrancyGuard contract from OpenZeppelin: Add the following import statement at the top of your contract file:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; Inherit from the ReentrancyGuard contract: Modify the Claimer contract to inherit from ReentrancyGuard:

contract Claimer is ReentrancyGuard { // ... rest of the contract code } Add the nonReentrant modifier to the claimPrizes function: Apply the nonReentrant modifier to the claimPrizes function to ensure that it cannot be re-entered during its execution:

function claimPrizes( IClaimable _vault, uint8 _tier, address[] calldata _winners, uint32[][] calldata _prizeIndices, address _feeRecipient, uint256 _minFeePerClaim ) external nonReentrant returns (uint256 totalFees) { // Function code... } By applying the nonReentrant modifier, you prevent the _claim function and any function it calls from being invoked again before the first invocation is completed. This effectively mitigates the risk of a reentrancy attack, ensuring that the contract state remains consistent and secure.

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

no impact demonstrated

[nevillehuang]

Invalid, lacks required PoC for reentrancy issues/non-obvious issues per sherlock rules