Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Reentrancy Vulnerability in FeeDistributionBranch::claimFees

cheatc0d33

Summary

A reentrancy vulnerability exists in the claimFees function of the FeeDistributionBranch contract, where state updates occur before an external token transfer. This could allow an attacker to reenter and claim fees multiple times before the first claim completes.

The claimFees function performs state updates through accumulateActor before executing an external token transfer, violating the Checks-Effects-Interactions (CEI) pattern. This order of operations creates a reentrancy risk.

function claimFees(uint128 vaultId) external {
// 1. State Read
UD60x18 amountToClaimX18 = vault.wethRewardDistribution.getActorValueChange(actorId);
// 2. State Update
vault.wethRewardDistribution.accumulateActor(actorId);
​
// 3. External Call
IERC20(weth).safeTransfer(msg.sender, amountToClaim);
}

Attack Scenario:

  1. Attacker calls claimFees

  2. Contract updates reward state via accumulateActor

  3. Before safeTransfer completes, attacker reenters claimFees

  4. Original state updates are still valid, allowing another claim

  5. Process repeats until funds are drained

Impact

The impact of this vulnerability is severe. An attacker could repeatedly withdraw the same fee amount multiple times in a single transaction, potentially draining all available fees from the contract. This would also corrupt the reward accounting system, leading to an inaccurate record of distributed rewards and undermining the entire fee distribution mechanism. The economic damage could be significant, affecting both the protocol and legitimate users expecting their rightful fee distributions.

Proof of Concept

contract AttackerContract {
FeeDistributionBranch target;
uint128 vaultId;
function attack() external {
target.claimFees(vaultId);
}
function onERC20Received(address, uint256) external {
try target.claimFees(vaultId) { } catch { }
}
}

Recommended Fixes

  1. Add OpenZeppelin's ReentrancyGuard:

contract FeeDistributionBranch is EngineAccessControl, ReentrancyGuard {
function claimFees(uint128 vaultId) external nonReentrant {
// existing code
}
}

  1. Restructure to follow CEI pattern:

function claimFees(uint128 vaultId) external nonReentrant {
// Checks
require(vaultId != 0, "Invalid vault");
// Effects
uint256 amountToClaim = calculateClaimAmount(vaultId);
updateRewardState(vaultId);
// Interactions
IERC20(weth).safeTransfer(msg.sender, amountToClaim);
}

  1. Make Distribution library functions atomic:

function accumulateActor(Data storage self, bytes32 actorId) internal returns (SD59x18) {
Actor storage actor = self.actor[actorId];
SD59x18 valueChange = _getActorValueChange(self, actor);
_updateLastValuePerShare(self, actor, ud60x18(actor.shares));
return valueChange;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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