The Standard

Summary

The provided Solidity contract, LiquidationPoolManager.sol, appears to be a manager contract responsible for handling a liquidation pool and distributing fees. I also added some GitHub links for your references.

Vulnerability Details

Cross-Function Reentrancy Risk:

1. Involves reentrant calls between different functions within the same contract.

2. In the provided LiquidationPoolManager.sol contract, the distributeFees function itself does not contain explicit external calls. However, the potential Cross-Function Reentrancy risk arises from the fact that distributeFees is called within the runLiquidation function, and there may be external calls made before or after the invocation of distributeFees.

Proof of concept

Here is the relevant portion of the runLiquidation function:

        function runLiquidation(uint256 _tokenId) external {
                  ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
                  manager.liquidateVault(_tokenId);
        // Hypothetical external call in distributeFees
                 distributeFees();
        // If distributeFees has external calls before state changes, a reentrancy risk is introduced
                ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
        // Rest of the function...
                 }

Note :
The actual vulnerability may depend on the implementation of the distributeFees function in the LiquidationPoolManager contract. If it makes external calls before state changes, it could introduce a reentrancy risk.

Here's a hypothetical example that illustrates the potential issue:

                      function distributeFees() public {
             // Hypothetical external call before state changes
            // This external call could potentially introduce a reentrancy risk
                      someExternalContract.someFunction();
            // ... (existing code)
                       }

In this case, if someExternalContract.someFunction() contains external calls before updating the state, it could create an opportunity for reentrancy.

To ensure the security of the contract, it's important to carefully review the entire codebase, including functions called within other functions, to ensure that external calls are made after updating the contract state (following the "Checks-Effects-Interactions" pattern).

Impact

Reentrancy Risk:

1. If external calls are made before updating the contract state, it could lead to reentrancy vulnerabilities, potentially allowing an attacker to manipulate the flow of the contract execution.

Tools Used

The analysis is based on manual inspection. No automated tools were used for this review.

Recommendations

To mitigate the potential reentrancy risk in the runLiquidation function of the LiquidationPoolManager contract, you can follow the "Checks-Effects-Interactions" pattern and ensure that external calls are made after updating the contract state. Here's a suggested modification:

                          function runLiquidation(uint256 _tokenId) external {
                     // Step 1: Liquidate the vault
                          ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
                          manager.liquidateVault(_tokenId);

                    // Step 2: Distribute fees, ensuring no external calls before state changes
                          distributeFees();

                   // Step 3: Interact with the LiquidationPool contract
                          ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
                          ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](tokens.length);

                   // ... (rest of the function)
                            }

                           function distributeFees() internal {
                           IERC20 eurosToken = IERC20(EUROs);
                           uint256 _feesForPool = eurosToken.balanceOf(address(this)) * poolFeePercentage / HUNDRED_PC;
                           if (_feesForPool > 0) {
                           eurosToken.approve(pool, _feesForPool);

                  // Move the external call after updating the contract state
                           LiquidationPool(pool).distributeFees(_feesForPool);
                                 }
                          eurosToken.transfer(protocol, eurosToken.balanceOf(address(this)));
                           }

In this modification:

1. Checks: The distributeFees function ensures that no external calls are made before updating the contract state. The approval and external call are placed after performing checks and updating the state.

2. Effects: The contract state is updated (e.g., approval for the pool and the external call are made) after the necessary checks.

3. Interactions: External calls are made only after the contract state has been updated.

This modification aligns with the "Checks-Effects-Interactions" pattern, helping to prevent Cross-Function Reentrancy Vulnerabilities by ensuring that external calls occur at a safe point in the execution flow.