036.md

Glamorous Plum Baboon

High

Reentrancy Risk in _burnCollateralATokens, _liquidateATokens, and _burnDebtTokens

Summary: In the LiquidationLogic.sol, functions _burnCollateralATokens, _liquidateATokens, and _burnDebtTokens perform external calls (such as calling transferOnLiquidation and burn) that could potentially introduce reentrancy vulnerabilities. These external calls are made after internal state updates, which deviates from the "checks-effects-interactions" pattern. This ordering can be exploited if an attacker is able to invoke a malicious contract that can make recursive calls into the vulnerable contract, thereby altering its state unexpectedly before the function completes execution.

Impact: i, Reentrancy Exploitation: An attacker could potentially manipulate the contract’s state by making recursive calls during an external function call (e.g., transferOnLiquidation or burn). ii, Funds Drainage: Malicious external contracts could exploit the vulnerability to drain assets, such as tokens or collateral, from the contract. iii, Data Inconsistencies: Malicious code could lead to inconsistent contract states, possibly corrupting balances, leading to faulty liquidations or token burns.

Root Cause: The root cause of this vulnerability is the improper ordering of actions within the functions. By performing external calls after modifying contract state variables (such as burning tokens or transferring collateral), the contract allows for the possibility of an external contract calling back into the vulnerable contract before the internal state changes are finalized. This is a direct violation of the "checks-effects-interactions" pattern, which is the standard defense against reentrancy attacks.

https://github.com/sherlock-audit/2025-01-aave-v3-3/blob/main/aave-v3-origin/src/contracts/protocol/libraries/logic/LiquidationLogic.sol#L443-L464

https://github.com/sherlock-audit/2025-01-aave-v3-3/blob/main/aave-v3-origin/src/contracts/protocol/libraries/logic/LiquidationLogic.sol#L528-L726

Proof of Concept (PoC):

1. Consider a scenario where the transferOnLiquidation function is calling a malicious contract.
2. The malicious contract contains a fallback function that makes another call to one of the vulnerable functions (_burnCollateralATokens, _liquidateATokens, or _burnDebtTokens).
3. By doing this, the malicious contract can recursively invoke these functions before the internal state is fully updated, potentially draining funds or leaving the contract in an inconsistent state.

Mitigation: To mitigate the reentrancy risk, the following changes are recommended:

1. Follow the Checks-Effects-Interactions Pattern: Ensure that all state-changing operations (such as balance adjustments, token burns, and collateral transfers) are performed before making any external calls.
   • Example:

     // Step 1: Perform internal state changes
     collateralBalance[msg.sender] -= amount;
     
     // Step 2: Perform external calls after state changes
     transferOnLiquidation(msg.sender, amount);

2. Implement a Reentrancy Guard: Use a reentrancy guard modifier (such as OpenZeppelin's nonReentrant) to prevent reentrancy attacks by blocking functions from being called recursively.