Irissme - Lack of nonReentrant Modifier in External Contract Interactions

[sherlock-admin]

Irissme

high

Lack of nonReentrant Modifier in External Contract Interactions

Summary

The smart contract lacks the use of the nonReentrant modifier in functions that interact with external contracts, such as approveTrade and others. This absence may expose the contract to potential reentrancy attacks.

Vulnerability Detail

The missing nonReentrant modifier in relevant functions allows an external malicious contract to recursively call these functions, potentially leading to reentrancy attacks. This can result in unexpected behavior and manipulation of the contract's state.

Impact

Consider the following hypothetical scenario where the approveTrade function lacks the nonReentrant modifier:

function approveTrade(address orderSender, bytes calldata tradeData) external onlyRegisteredPerp {
    // ... existing code ...
    (Types.Order[] memory orderList, bytes[] memory signatureList, uint256[] memory matchPaperAmount) =
        abi.decode(tradeData, (Types.Order[], bytes[], uint256[]));
    // ... existing code ...

    // Malicious external contract exploiting reentrancy
    maliciousContract.executeReentrancyAttack();
    // ... additional code ...
}

In this example, an attacker deploys a malicious contract (maliciousContract) designed to execute a reentrancy attack. The malicious contract has a function named executeReentrancyAttack, which repeatedly calls back into the vulnerable contract's approveTrade function.

Without the nonReentrant modifier, the attacker can exploit this vulnerability to repeatedly enter the approveTrade function before it completes its execution. This opens the door to manipulations of the contract state during ongoing transactions, allowing the attacker to:

Front-Run Trades: Execute trades ahead of legitimate traders, taking advantage of price fluctuations. Steal Funds: Withdraw assets or manipulate balances during the vulnerable state. Manipulate State: Alter contract state unpredictably, leading to unintended outcomes. By introducing the nonReentrant modifier, these types of attacks are mitigated. The modifier prevents the approveTrade function from being reentered during its execution, significantly enhancing the security of the contract against reentrancy vulnerabilities.

Code Snippet

https://github.com/sherlock-audit/2023-12-jojo-exchange-update/blob/main/smart-contract-EVM/src/JOJOExternal.sol#L100-L103

Tool used

Manual Review

Recommendation

// Updated code snippet with nonReentrant modifier
modifier nonReentrant() {
    require(!state.reentrantGuard, "ReentrancyGuard: reentrant call");
    state.reentrantGuard = true;
    _;
    state.reentrantGuard = false;
}

// Modified function with nonReentrant modifier
function approveTrade(address orderSender, bytes calldata tradeData) external onlyRegisteredPerp nonReentrant {
    // ... existing code ...
    (Types.Order[] memory orderList, bytes[] memory signatureList, uint256[] memory matchPaperAmount) =
        abi.decode(tradeData, (Types.Order[], bytes[], uint256[]));
    // ... existing code ...
}

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { invalid}

[nevillehuang]

Invalid, insufficient proof that reentrancy is possible