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

Dependence on External Calls

mustaphaabdulaziz00

Summary

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/market-making/branches/FeeDistributionBranch.sol#L284

The FeeDistributionBranch contract relies on external calls, such as token transfers using the ERC-20 transfer function. This dependence introduces risks of reentrancy, unexpected failures, and gas consumption issues.

Vulnerability Details

The claimFees function interacts directly with external contracts via the IERC20.transfer call:

function claimFees(address[] memory assets) external {
for (uint256 i = 0; i < assets.length; i++) {
uint256 amount = feeBalances[assets[i]];
if (amount > 0) {
feeBalances[assets[i]] = 0;
IERC20(assets[i]).transfer(msg.sender, amount);
emit FeesClaimed(msg.sender, assets[i], amount);
}
}
}

if an external token’s transfer function is overridden to include malicious logic, it could reenter the contract before state changes (e.g., updating feeBalances) are finalized.

Unexpected Failures: External calls may fail for various reasons, such as insufficient gas or non-standard token implementations.

Gas Wastage: A loop making multiple external calls (e.g., one per asset) can result in high gas consumption, especially with maliciously large input arrays.

Impact

An attacker could exploit the claimFees function by reentering the contract, causing inconsistent state updates or draining funds.

Tools Used

Manual review

Recommendations

Adopt the Checks-Effects-Interactions Pattern:

Update the contract state before making any external calls to prevent reentrancy.

function claimFees(address[] memory assets) external {
for (uint256 i = 0; i < assets.length; i++) {
uint256 amount = feeBalances[assets[i]];
if (amount > 0) {
feeBalances[assets[i]] = 0; // Update state first
require(IERC20(assets[i]).transfer(msg.sender, amount), "Transfer failed");
emit FeesClaimed(msg.sender, assets[i], amount);
}
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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