0xVinylDavyl - The code utilizes low-level calls in the `_commitPrice` function, In the context of this function, the contract's behavior depends on the success of this low-level call, which introduces potential risks.

[sherlock-admin2]

0xVinylDavyl

high

The code utilizes low-level calls in the _commitPrice function, In the context of this function, the contract's behavior depends on the success of this low-level call, which introduces potential risks.

Summary

The code utilizes low-level calls in the _commitPrice function, which can be risky and may lead to vulnerabilities if not used with caution.

Vulnerability Detail

The vulnerability lies in the usage of low-level calls, specifically in the _commitPrice function. Low-level calls like call or delegatecall can be challenging to use correctly and securely. In the context of this function, the contract's behavior depends on the success of this low-level call, which introduces potential risks.

Impact

The usage of low-level calls without proper handling may result in unexpected behavior and vulnerabilities. If the low-level call fails or behaves unexpectedly, it can leave the contract in an inconsistent state.

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-extensions/contracts/MultiInvoker.sol#L312

function _commitPrice(
    address oracleProvider,
    uint256 value,
    uint256 index,
    uint256 version,
    bytes memory data,
    bool revertOnFailure
) internal {
    UFixed18 balanceBefore = DSU.balanceOf();

    if (revertOnFailure) {
        IPythOracle(oracleProvider).commit{value: value}(index, version, data);
    } else {
        try IPythOracle(oracleProvider).commit{value: value}(index, version, data) { }
        catch { }
    }

    // Return through keeper reward if any
    DSU.push(msg.sender, DSU.balanceOf().sub(balanceBefore));
}

Tool used

Manual Review

Recommendation

To mitigate the risks associated with low-level calls, it is recommended to consider using higher-level functions or libraries that provide more safety when interacting with external contracts. A potential improvement can be achieved by using the OpenZeppelin library's Address utility, which includes functions for interacting with external contracts safely.

Here's an example of how to use the Address utility:

import "@openzeppelin/contracts/utils/Address.sol";

function _commitPrice(
    address oracleProvider,
    uint256 value,
    uint256 index,
    uint256 version,
    bytes memory data,
    bool revertOnFailure
) internal {
    UFixed18 balanceBefore = DSU.balanceOf();

    // Use Address function to perform a call and handle exceptions gracefully
    bool success = Address.call{value: value}(oracleProvider, abi.encodeWithSelector(IPythOracle.commit.selector, index, version, data));

    // Check for success or revert based on the revertOnFailure flag
    if (revertOnFailure && !success) {
        revert("Commit failed");
    }

    // Return through keeper reward if any
    DSU.push(msg.sender, DSU.balanceOf().sub(balanceBefore));
}

By using the Address utility, you can perform calls in a safer manner, handle exceptions gracefully, and have better control over the contract's behavior in case of failures.

[sherlock-admin]

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

panprog commented:

invalid because there is no issue and no real impact or scenario presented

n33k commented:

invalid, ai generated groundless report

darkart commented:

AI

polarzero commented:

Invalid. While this is a legitimate concern, this would only be misleading, and would not impact the good functioning of the protocol.