Reentrancy vulnerability in Vesting contract on claim
Summary
The claim method in the Vesting contract is vulnerable to a reentrancy attack due to its handling of external calls before updating the index user's claim status. This flaw could potentially allow attackers to drain the vesting pool by claiming their vested tokens multiple times.
Vulnerability Detail
The vulnerability exists in the claim method, specifically when transferring native tokens (e.g., ETH) or tokens that utilize hooks which may call back into the contract. The external call to transfer tokens to the sender (payable(sender).call{value: pctAmount}("") or token.safeTransfer(sender, pctAmount)) occurs before updating the claim index (s.index = uint128(i)). This ordering allows for a possibility of reentrancy where the attacker re-enters the claim function before the index is updated, leading to multiple unauthorized claims.
Impact
An attacker could exploit this vulnerability to repeatedly claim tokens beyond their vested amount, potentially draining the funds allocated for vesting. This not only affects the integrity of the vesting process but could also result in significant financial loss to the stakeholders.
To mitigate this vulnerability, consider reordering the operations in the claim function to update the user's claim status (s.index = uint128(i)) before making any external calls, implementing checks-effects-interactions pattern more strictly could help in preventing such reentrancy attacks.
Using a reentrancy guard (e.g., OpenZeppelin's ReentrancyGuard) is also recommended to further secure the contract against reentrancy.
0x4non
high
Reentrancy vulnerability in Vesting contract on
claimSummary
The
claimmethod in the Vesting contract is vulnerable to a reentrancy attack due to its handling of external calls before updating the index user's claim status. This flaw could potentially allow attackers to drain the vesting pool by claiming their vested tokens multiple times.Vulnerability Detail
The vulnerability exists in the
claimmethod, specifically when transferring native tokens (e.g., ETH) or tokens that utilize hooks which may call back into the contract. The external call to transfer tokens to the sender (payable(sender).call{value: pctAmount}("")ortoken.safeTransfer(sender, pctAmount)) occurs before updating the claim index (s.index = uint128(i)). This ordering allows for a possibility of reentrancy where the attacker re-enters theclaimfunction before the index is updated, leading to multiple unauthorized claims.Impact
An attacker could exploit this vulnerability to repeatedly claim tokens beyond their vested amount, potentially draining the funds allocated for vesting. This not only affects the integrity of the vesting process but could also result in significant financial loss to the stakeholders.
Code Snippet
https://github.com/sherlock-audit/2024-03-zap-protocol/blob/main/zap-contracts-labs/contracts/Vesting.sol#L82-L91
Tool used
Manual Review
Recommendation
To mitigate this vulnerability, consider reordering the operations in the claim function to update the user's claim status (s.index = uint128(i)) before making any external calls, implementing checks-effects-interactions pattern more strictly could help in preventing such reentrancy attacks. Using a reentrancy guard (e.g., OpenZeppelin's ReentrancyGuard) is also recommended to further secure the contract against reentrancy.
Duplicate of #157