w42d3n - Reentrancies in oracle.sol

[sherlock-admin2]

w42d3n

medium

Reentrancies in oracle.sol

Summary

The contract oracle.sol does not protect against reentrancy attacks as it does not use the nonReentrant modifier on public and external functions that invoke other contract functions using an external call.

Vulnerability Detail

In particular, update and request functions call another contract function _updateCurrent and _updateLatest which can lead to potential reentrancy attacks if the referenced contract's functions are malicious.

Impact

This vulnerability allows an attacker to drain a contract by repeatedly invoking the function before the initial function invocation has finished.

Code Snippet

https://github.com/sherlock-audit/2024-02-perennial-v2-3/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L30-L47

/// @notice Updates the current oracle provider
/// @dev Both the current and new oracle provider must have the same current
/// @param newProvider The new oracle provider
function update(IOracleProvider newProvider) external onlyFactory {
    _updateCurrent(newProvider);
    _updateLatest(newProvider.latest());
}

/// @notice Requests a new version at the current timestamp
/// @param market Original market to optionally use for callbacks
/// @param account Original sender to optionally use for callbacks
function request(IMarket market, address account) external onlyAuthorized {
    (OracleVersion memory latestVersion, uint256 currentTimestamp) = oracles[global.current].provider.status();

    oracles[
        (currentTimestamp > oracles[global.latest].timestamp) ? global.current : global.latest
    ].provider.request(market, account);

    oracles[global.current].timestamp = uint96(currentTimestamp);
    _updateLatest(latestVersion);
}

Tool used

Manual Review

Recommendation

To mitigate this issue, it is a good practice for Solidity developers to make all external function calls at the end of the function to minimize the malicious possibilities of external calls. Another important mitigation is to use a nonReentrant modifier on these functions, this modifier is usually provided by OpenZeppelin's ReentrancyGuard.

[sherlock-admin3]

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

panprog commented:

invalid, it's impossible to reenter these functions as there are no external calls

[nevillehuang]

Invalid, lack required PoC by sherlock for reentrancy