safdie - Reentrancy attack allow an attacker to drain funds by repeatedly calling the liquidation function in `LibLiquidation.sol`

[sherlock-admin3]

safdie

High

Reentrancy attack allow an attacker to drain funds by repeatedly calling the liquidation function in LibLiquidation.sol

Summary

Reentrancy attack in LibLiquidation.sol allow an attacker to drain funds by repeatedly calling the liquidation function.

Root Cause

There is a possibility of a reentrancy attack when liquidator rewards (liquidatorShare) are transferred at the end of the function: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibLiquidation.sol#L91-L92 If a malicious contract or address is the liquidator, it may attempt to re-enter the liquidation process by triggering a function call from within the event emission or balance transfer. In the context of the LibLiquidation contract, there could be a potential reentrancy vulnerability if external calls are made while a contract’s state is only partially updated, especially if user balances or funds are involved.

In the LibLiquidation contract, we see several areas that interact with user balances and funds. For instance, the following portion of code is interacting with balances and transferring funds to the liquidator: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibLiquidation.sol#L90-L93

If there are any external calls made to another contract or to the liquidator during the liquidation process, and if the contract does not update the liquidation status or user balances before making these external calls, a malicious user could re-enter the function multiple times to exploit it. So, if msg.sender (the liquidator) is a contract that has a fallback function or a malicious contract, it could potentially re-enter the liquidatePartyB function. If the contract state (like user balances) isn’t properly updated before the external call, the attacker could continuously liquidate and drain more funds than they should be entitled to.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. The vulnerable contract holds the initial balances. Let’s assume Party A has 100 ETH in their balance. The attacker deploys the ReentrancyAttack contract with the vulnerable liquidation contract's address.
2. The attacker calls the attack() function in their contract. The liquidate() function in the vulnerable contract transfers 1 ETH to the attacker's contract. The attacker's contract receives the ETH and its receive() function is triggered. This function re-enters the vulnerable contract by calling liquidate() again before the previous state change (balance deduction) is completed.
3. The attacker re-enters the liquidation function multiple times, draining the vulnerable contract’s balance. Each time, the vulnerable contract transfers 1 ETH to the attacker without deducting the correct amount from Party A’s balance until the re-entrance stops.

Impact

1. The attacker can drain all funds from the contract by recursively re-entering the function before the balance is properly deducted. Even though Party A had a large balance, the attacker can repeatedly liquidate and take more funds than they were entitled to.
2. All users whose balances are managed by the vulnerable contract can lose funds, and the liquidator (attacker) can drain the entire contract balance. Depending on the contract size, this could result in significant financial loss, especially in cases where real-world assets are represented by the smart contract.
3. Liquidation processes or other important financial workflows can be disrupted, leading to unfair payouts or invalid liquidation statuses. This can damage the integrity of the platform and result in a loss of user trust.

PoC

Here's a simplified PoC demonstrating how a reentrancy attack could be exploited in a liquidation-like contract where external calls are involved: Vulnerable code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;

contract LiquidationVulnerable {
    mapping(address => uint256) public balances;

    // Constructor to initialize balances
    constructor() {
        balances[msg.sender] = 100 ether; // Party A starts with 100 ETH
    }

    function liquidate(address liquidator) public {
        uint256 liquidatorShare = 1 ether; // Liquidator gets 1 ETH for liquidation
        require(balances[msg.sender] >= liquidatorShare, "Insufficient balance");

        // Simulate external call to a liquidator contract (this is dangerous)
        (bool success, ) = liquidator.call{value: liquidatorShare}("");
        require(success, "Transfer to liquidator failed");

        // Deduct the liquidator's share from sender's balance
        balances[msg.sender] -= liquidatorShare;
    }

    // Fallback function to receive ETH
    receive() external payable {}
}

Attacker contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;

contract ReentrancyAttack {
    LiquidationVulnerable public vulnerableContract;

    constructor(address _vulnerableContract) {
        vulnerableContract = LiquidationVulnerable(_vulnerableContract);
    }

    // Attack function that starts the liquidation and re-enters the vulnerable contract
    function attack() public {
        vulnerableContract.liquidate(address(this));
    }

    // Fallback function that is called when receiving ETH, re-enters the liquidation function
    receive() external payable {
        if (address(vulnerableContract).balance > 1 ether) {
            vulnerableContract.liquidate(address(this)); // Re-enter the liquidation function
        }
    }
}

Mitigation

1. Introduce a reentrancy guard (such as the nonReentrant modifier from OpenZeppelin’s library) to prevent recursive calls during sensitive operations. Example of a reentrancy guard:

   
   uint256 private _status;

modifier nonReentrant() { require(_status != 1, "ReentrancyGuard: reentrant call"); status = 1; ; _status = 0; }

Apply it to critical functions:
```solidity
function liquidate(address liquidator) public nonReentrant {
    // function logic
}

2. Always update the contract’s state (e.g., deduct balances, mark users as liquidated, etc.) before making any external calls. This prevents the contract from being in an inconsistent state during the execution.
3. Where possible, avoid making direct call, delegatecall, or transfer calls to untrusted addresses or contracts during critical logic execution. Instead, consider more secure patterns like allowing users to withdraw funds by explicitly calling a function themselves.