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

Reentrancy Vulnerability in LiquidationPoolManager Contract

hackj5

Summary

The code appears to have a potential reentrancy vulnerability in the runLiquidation function, which could lead to unexpected behavior and security risks.

Vulnerability Details

In the runLiquidation function, after calling manager.liquidateVault(_tokenId), the distributeFees function is invoked. However, this function interacts with external contracts and transfers tokens, creating a potential reentrancy vulnerability. If any of the external contracts involved in distributeFees trigger a callback to this contract, the distributeFees function may be reentered before completing, leading to unexpected and potentially malicious behavior.

Impact

If exploited, the reentrancy vulnerability could result in unexpected state changes and potential security risks. It is crucial to address this issue promptly to safeguard the integrity and security of the contract.

Tools Used

Manual Review

Recommendations

Implement reentrancy protection to ensure that the contract state is correctly managed during external calls. One common practice is to use the "Checks-Effects-Interactions" pattern where external calls are made after state changes. Here is a suggestion for updating the runLiquidation function:

function runLiquidation(uint256 _tokenId) external {
ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
manager.liquidateVault(_tokenId);
// Ensure reentrancy protection
bool reentrancyGuard;
require(!reentrancyGuard, "Reentrancy protection");
​
distributeFees();
ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](tokens.length);
uint256 ethBalance;
​
for (uint256 i = 0; i < tokens.length; i++) {
ITokenManager.Token memory token = tokens[i];
if (token.addr == address(0)) {
ethBalance = address(this).balance;
if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
} else {
IERC20 ierc20 = IERC20(token.addr);
uint256 erc20balance = ierc20.balanceOf(address(this));
if (erc20balance > 0) {
assets[i] = ILiquidationPoolManager.Asset(token, erc20balance);
ierc20.approve(pool, erc20balance);
}
}
}
// Set reentrancy guard
reentrancyGuard = true;
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
// Reset reentrancy guard
reentrancyGuard = false;
​
forwardRemainingRewards(tokens);
}

By using a reentrancy guard, you can mitigate the risk of reentrancy attacks during the execution of the distributeFees and distributeAssets functions.

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.