sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

0xVinylDavyl - Potential Reentrancy Vulnerability concerned in the contract's fund function, which interacts with an external contract (market.claimFee()) without implementing a reentrancy guard. Potentially exposing the contract to reentrancy attacks #16

Closed sherlock-admin closed 11 months ago

sherlock-admin commented 11 months ago

0xVinylDavyl

medium

Potential Reentrancy Vulnerability concerned in the contract's fund function, which interacts with an external contract (market.claimFee()) without implementing a reentrancy guard. Potentially exposing the contract to reentrancy attacks

Summary

The "Potential Reentrancy Vulnerability" is a security concern in the contract's fund function, which interacts with an external contract (market.claimFee()) without implementing a reentrancy guard. This vulnerability can potentially expose the contract to reentrancy attacks, allowing malicious code in the external contract to perform unexpected actions.

Vulnerability Detail

The vulnerability arises from the contract's interaction with external contracts without protecting against reentrancy attacks. Reentrancy occurs when an external contract calls back into a vulnerable contract before the first call completes, potentially allowing malicious external code to manipulate the contract's state.

Impact

If an external contract (e.g., market) calls back into the fund function while it is executing market.claimFee(), an attacker could exploit this vulnerability to perform unauthorized actions, manipulate state variables, or even drain the contract's funds.

Code Snippet

// Potential Reentrancy Vulnerability
function fund(IMarket market) external {
    if (!instances(IInstance(address(market.oracle())))) revert FactoryNotInstanceError();
    market.claimFee();
}

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/OracleFactory.sol#L111

Tool used

Manual Review

Recommendation

To mitigate the potential reentrancy vulnerability, follow these recommended steps:

  1. Implement a Reentrancy Guard: Implement a reentrancy guard in functions that interact with external contracts. You can use the OpenZeppelin ReentrancyGuard library to add protection against reentrancy attacks. Here's an example of how to modify the code:

    // Import the ReentrancyGuard library
    import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
    
    // Add the ReentrancyGuard contract to your contract inheritance
    contract OracleFactory is IOracleFactory, Factory, ReentrancyGuard {
       // ...
    
       function fund(IMarket market) external nonReentrant {
           if (!instances(IInstance(address(market.oracle())))) revert FactoryNotInstanceError();
           market.claimFee();
       }
    }
  2. Ensure External Contracts Follow Best Practices: Verify that external contracts (market in this case) adhere to best practices to prevent reentrancy vulnerabilities. Contracts interacting with your contract should also have proper access controls and reentrancy protection where necessary.

Implementing these recommendations will help protect your contract against reentrancy attacks and enhance its security.

sherlock-admin commented 11 months ago

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

panprog commented:

invalid because market can not be used to re-enter and no real scenario of reentrancy is presented (possible reentrancy is not a valid issue)

n33k commented:

invalid, ai generated groundless report

0xyPhilic commented:

invalid because there is no PoC confirming a possible re-entrancy

polarzero commented:

Invalid. This is not clear what could actually happen in this scenario; the issue lacks concrete explanation and proof of concept.