search

sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

0x_Sanzcy - Non refundable Fees sent to pay the Pyth off-chain oracle will be locked in the contract #94

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

0x_Sanzcy

medium

Non refundable Fees sent to pay the Pyth off-chain oracle will be locked in the contract

Summary

updatePythPrice function takes a fee when users update the price data, the fees is sent along with the call as msg.value and the fees is deducted from it, if there's any left over value after the call it is refunded back to the user. The issue is when the user have a fallback function to reject all incoming Eth the user won't be able to receive the refund and there's no way to retrieve such failed transaction Eth from the oracleModule contract.

Vulnerability Detail

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();

The function is marked payable indicating the function can receive ETH, the fees is then calculated in the getUpdateFee function which returns the amount of fee the user will pay for the update.

If the user sent more Eth than required for the fee the rest is sent back to the sender 

    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();

Which reverts on failure if the refund fails.

Impact

Non refundable Eth will remain locked on the contract

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1%2Fsrc%2FOracleModule.sol#L64-L76

Tool used

Manual Review

Recommendation

Include a function to retrieve nonrefundable Eth from the contract

sherlock-admin commented 8 months ago

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

takarez commented:

invalid: this is a loss for user and that he should have a fallback function

nevillehuang commented 8 months ago

Invalid, user error not valid based on sherlock rules. It is keepers responsibility to ensure the contract/EOA they are calling updatePythPrice() from can receive the refunds appropriately