Possible Redirect of Refunds Due to Configurable Sender Parameter in updatePythPrice Function

Summary

There is a potential misdirection of refunds due to the configurable sender parameter in the updatePythPrice function OracleModule.sol:64-76. The function attempts to refund any excess ether sent by calling sender.call{value: msg.value - fee}(""). However, the sender parameter is not explicitly verified or authenticated in any way, leaving open the possibility that the refund could be sent to an unintended recipient.

To mitigate this potential issue, it's crucial to verify the sender parameter before executing any refund operation. This can be achieved by implementing proper access control mechanisms or by ensuring that only trusted addresses are allowed to receive refunds.

Vulnerability Detail

The scenario to exploit the potential misdirection of refunds in the updatePythPrice function involves an attacker manipulating the sender parameter to redirect the refund to an unintended recipient. In this scenario, the victim would typically be a user who interacts with the updatePythPrice function with the expectation of receiving a refund if they've overpaid. Here's a possible scenario:

Attacker Interaction: The attacker deploys a malicious Web3 frontend interface that interacts with the updatePythPrice function.
User Manipulation: A user interacts with the malicious Web3 frontend interface to operate on the legitimate smart contract project. However, the malicious Web3 frontend interface calls updatePythPrice with its own address as the sender parameter.
Function Execution: The function executes, and the attacker's address is considered as the sender.
Refund Misdirection: After deducting the fee, the function attempts to refund any excess ether to the sender (which is the attacker's address).
Exploit Outcome: The attacker successfully redirects the refund to their own address, potentially causing financial loss or disrupting the intended flow of funds.

Impact

By manipulating the sender parameter and exploiting the lack of refunds back enforcement, the attacker has the possibility to deceive the user into sending the refund to an unintended recipient. This scenario underscores the importance of enforcing funds workflow and not leaving free will to the user to select the addresses, allowing malicious actors to redirect the flow of funds.

Code Snippet

OracleModule.sol:64-76

    function updatePythPrice(address sender, bytes[] calldata priceUpdateData) external payable nonReentrant {
        // Get fee amount to pay to Pyth
        uint256 fee = offchainOracle.oracleContract.getUpdateFee(priceUpdateData);

        // Update the price data (and pay the fee)
        offchainOracle.oracleContract.updatePriceFeeds{value: fee}(priceUpdateData);

        if (msg.value - fee > 0) {
            // Need to refund caller. Try to return unused value, or revert if failed
            (bool success, ) = sender.call{value: msg.value - fee}("");
            if (success == false) revert FlatcoinErrors.RefundFailed();
        }
    }

Tool used

Manual Review

Recommendation

Ensure that refunds are directed back to the original caller by verifying that the sender parameter corresponds to the msg.sender or by enforcing the refunds to the caller.

verifying that the sender parameter corresponds to the msg.sender


function updatePythPrice(address sender, bytes[] calldata priceUpdateData) external payable nonReentrant {

require(msg.sender == sender, "Refunds must be returned to the caller."); // Get fee amount to pay to Pyth uint256 fee = offchainOracle.oracleContract.getUpdateFee(priceUpdateData);

// Update the price data (and pay the fee)
offchainOracle.oracleContract.updatePriceFeeds{value: fee}(priceUpdateData);

if (msg.value - fee > 0) {
    // Need to refund caller. Try to return unused value, or revert if failed
    (bool success, ) = sender.call{value: msg.value - fee}("");
    if (success == false) revert FlatcoinErrors.RefundFailed();
}

}

enforcing the refunds to the caller
function updatePythPrice(address sender, bytes[] calldata priceUpdateData) external payable nonReentrant {
function updatePythPrice(bytes[] calldata priceUpdateData) external payable nonReentrant { // Get fee amount to pay to Pyth uint256 fee = offchainOracle.oracleContract.getUpdateFee(priceUpdateData);

// Update the price data (and pay the fee) offchainOracle.oracleContract.updatePriceFeeds{value: fee}(priceUpdateData);

if (msg.value - fee > 0) { // Need to refund caller. Try to return unused value, or revert if failed
(bool success, ) = sender.call{value: msg.value - fee}("");
(bool success, ) = msg.sender.call{value: msg.value - fee}(""); if (success == false) revert FlatcoinErrors.RefundFailed(); } }

Presently, the majority of updatePythPrice function calls include msg.sender as a parameter. Therefore, It seems prudent to mandate this parameter's enforcement rather than maintaining its configurability.

This is my first issue ever, I hope it helps. Therefore, I'm here to learn and growth. Thank you.

sherlock-admin commented 7 months ago

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

takarez commented:

invalid

nevillehuang commented 7 months ago

Invalid, caller/keeper updating price responsible for inputting correct address to receive refunded funds.

sherlock-audit / 2023-12-flatmoney-judging

toufik-airane - Possible Redirect of Refunds Due to Configurable Sender Parameter in updatePythPrice Function #243