BZ - Unprotected Ether Transfer in `OracleModule.updatePythPrice` Resulting to Loss of Funds and Unexpected Failures

[sherlock-admin2]

BZ

high

Unprotected Ether Transfer in OracleModule.updatePythPrice Resulting to Loss of Funds and Unexpected Failures

Summary

The function sends Ether back to an arbitrary user (the sender parameter), which could be any Ethereum address. This refund mechanism introduces risk, as there is no guarantee that the recipient address is capable of handling incoming Ether transfers safely.

If the refund is sent to a contract address that does not handle incoming Ether properly (e.g., if it is a smart contract that lacks a payable function or an appropriate fallback function), the Ether transfer may fail and potentially lock the funds, causing them to be lost.

Moreover, the unrestricted refunding of Ether to arbitrary addresses can inadvertently introduce reentrancy issues if the receiving address is a malicious contract that triggers a reentrancy attack through a callback function.

Vulnerability Detail

The updatePythPrice function in the OracleModule contract contains a call to the offchain oracle's updatePriceFeeds function with a specified fee. After updating the price data and paying the required fee, the function then attempts to refund the remaining balance back to the caller (sender) through a direct Ether transfer.

This functionality poses a risk as it allows the transfer of Ether to an arbitrary address, which could be controlled by an attacker. If the recipient is a malicious contract, it could exploit the transfer in ways that may lead to unexpected behavior.

Impact

Loss of Funds: If the Ether refund fails, it could lead to loss of funds that cannot be recovered. Reentrancy Attack: The function is protected by a reentrancy guard, but the arbitrary refund could still open the door to reentrancy attacks from malicious contracts. Unexpected Failures: Sending Ether to arbitrary addresses may result in unexpected failures if the receiving contract does not support Ether transfers, disrupting the intended operation of the contract.

Code Snippet

https://github.com/sherlock-audit/2024-03-flat-money-fix-review-contest/blob/main/flatcoin-v1/src/OracleModule.sol#L58

Exploit Scenario

Suppose there is a malicious contract, MaliciousContract, that sets itself as the recipient for the refund in the uploit Scenario:datePythPrice function. The contract could execute the following steps:

1.  `Deploy MaliciousContract.`
2.  Call the `updatePythPrice` function on the OracleModule contract, passing a price update data parameter and value in Ether.
3.  The `updatePythPrice` function deducts the fee and tries to refund the remaining Ether to the caller (`MaliciousContract`).
4.  When `MaliciousContract` receives the Ether, it could exploit the refund by:
    Triggering a reentrancy attack on the `OracleModule` contract.
    Attempting to transfer the received Ether to another address, potentially draining the balance from `OracleModule`.

Proof of Concept

1. Go to remix.ethereum.org
   2. Deploy the OracleModule.sol contract
   3. Deploy The below MaliciousContract.sol

 // SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "./OracleModule.sol";  // Adjust the import path as necessary

contract MaliciousContract {
    event ReceivedEther(uint256 amount);
    OracleModule public oracleModule;
    address public owner;

    constructor(address _oracleModule) {
        oracleModule = OracleModule(_oracleModule);
        owner = msg.sender;
    }

    // Function to receive Ether
    receive() external payable {
        emit ReceivedEther(msg.value);
        // Additional malicious logic can be executed here
    }

    // Function to execute the attack
    function executeAttack(bytes[] calldata priceUpdateData, uint256 fee) external payable {
        // Call updatePythPrice with this contract's address as the sender and the supplied data and fee
        oracleModule.updatePythPrice{value: msg.value}(address(this), priceUpdateData);

        // At this point, the refund will be sent to this contract's address
    }
}

Tool used

Manual Review

Recommendation

Ensure proper checks and balances in the updatePythPrice function before transferring Ether. Consider using transfer or send with proper error handling or using a withdrawal pattern. Alternatively, consider specifying a trusted address to refund Ether, instead of using an arbitrary address.