The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

there's a risk of reentrancy attacks due to the order of operations

0xluckyluke

Summary

The issue is in the LiquidationPool.distributeFees() function, where there's a risk of reentrancy attacks due to the order of operations in the distributeFees function.

Vulnerability Details

The LiquidationPool.distributeFees() function updates state variables after interacting with external contracts, making it vulnerable to reentrancy attacks. An attacker could exploit this by repeatedly calling the function before state variables are updated.

Impact

This vulnerability may allow unexpected behavior, as an attacker could manipulate the contract's state during the execution of decreasePosition.

Tools Used

Slither

Recommendations

  1. Reorder Operations:
    Change the order of operations in distributeFees to update state variables before making external calls, reducing the risk of reentrancy attacks.

function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
// change order
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
}
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
}
}

  1. Use Checks-Effects-Interactions Pattern:
    Follow a pattern where checks and state variable updates come before interactions with external contracts to minimize reentrancy risks.

// Example of Checks-Effects-Interactions pattern
if (tstTotal > 0) {
// Checks
for (uint256 i = 0; i < holders.length; i++) {
// Effects
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
// Interactions
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
}

By making these changes, the contract can be more secure against reentrancy attacks.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic
Assigned finding tags:

informational/invalid

Support

FAQs

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