The Standard

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

The manager.call{value: _assets[i].amount - _nativePurchased}(""); line in the returnUnpurchasedNative function performs an unchecked external call, leading potential exploit

Summary

The manager.call{value: _assets[i].amount - _nativePurchased}(""); line in the returnUnpurchasedNative function performs an unchecked external call, which may lead to unexpected behavior and could potentially be exploited.

An attacker may manipulate the call to exploit vulnerabilities or interfere with the contract's intended behavior.

Vulnerability Details

Code Snippet:

(bool _sent, ) = manager.call{value: _assets[i].amount - _nativePurchased}("");

The manager.call{value: _assets[i].amount - _nativePurchased}(""); line in the returnUnpurchasedNative function performs an unchecked external call.

Impact

Performing an unchecked external call can result in unexpected behavior and poses a security risk. If the external call fails or the target address (manager in this case) reverts, the contract execution continues without any error handling. This may lead to an inconsistent state in the contract or even financial losses.

Tools Used

Manual Code Review

Recommendations

To address the unchecked external call in the returnUnpurchasedNative function, the following recommendations are provided:

  1. Check External Call Success:

    • Ensure that external calls are checked for success by capturing the return value and verifying that it is true. This helps prevent unintended consequences in case the external call fails.

    (bool _sent, ) = manager.call{value: _assets[i].amount - _nativePurchased}("");
    require(_sent, "external call failed");
  2. Implement ReentrancyGuard:

    • Consider implementing the ReentrancyGuard pattern to mitigate reentrancy issues. This involves using the nonReentrant modifier to prevent reentrancy attacks during external calls.

    import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
    contract LiquidationPool is ILiquidationPool, ReentrancyGuardUpgradeable {
    // Existing contract code...
    function returnUnpurchasedNative(ILiquidationPoolManager.Asset[] memory _assets, uint256 _nativePurchased) private nonReentrant {
    for (uint256 i = 0; i < _assets.length; i++) {
    // Existing function logic...
    (bool _sent, ) = manager.call{value: _assets[i].amount - _nativePurchased}("");
    require(_sent, "external call failed");
    // Existing function logic...
    }
    }
    // Existing contract code...
    }

By implementing these recommendations, the contract can enhance security by ensuring proper handling of external calls and mitigating potential reentrancy vulnerabilities.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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