sherlock-audit / 2024-10-ethos-network-judging

0 stars 0 forks source link

aycozyOx - Failure to Handle Token Transfer with safeTransfer in _handlePayment Function #256

Open sherlock-admin2 opened 2 weeks ago

sherlock-admin2 commented 2 weeks ago

aycozyOx

Medium

Failure to Handle Token Transfer with safeTransfer in _handlePayment Function

Summary

The _handlePayment function in Ethosreview.sol fails to handle token transfers using safeTransfer. This can lead to potential issues with token transfers, including failed transactions and loss of funds.

Root Cause

The root cause of this issue is the use of transferFrom instead of safeTransferFrom for ERC-20 token transfers. The transferFrom function does not handle all possible failure scenarios, which can result in unsuccessful transfers without proper error handling.

https://github.com/sherlock-audit/2024-10-ethos-network/blob/main/ethos/packages/contracts/contracts/EthosReview.sol#L478-L494

  function _handlePayment(address paymentToken) private onlyValidPaymentToken(paymentToken) {
    uint256 price = reviewPrice[paymentToken].price;

    if (price > 0) {
      if (paymentToken == address(0)) {
        if (msg.value != price) {//Strict value
          revert WrongPaymentAmount(paymentToken, msg.value);
        }
      } else {
        if (msg.value > 0) {
          revert WrongPaymentAmount(address(0), msg.value);
        }

        IERC20(paymentToken).transferFrom(msg.sender, address(this), price);
      }
    }
  }

Internal pre-conditions

For ERC-20 token payments, the function attempts to transfer tokens from the sender to the contract using transferFrom, which is obviously the case in the _handlePayment() function

External pre-conditions

No response

Attack Path

-A user initiates a payment with an ERC-20 token. -The _handlePayment function calls transferFrom to transfer the tokens. -If the token transfer fails, the function does not handle the failure properly, leading to potential loss of funds or failed transactions.

Impact

The function may fail to transfer tokens without proper error handling, leading to failed transactions.

PoC

No response

Mitigation

Replace transferFrom with safeTransferFrom to ensure proper handling of token transfers and to revert the transaction in case of failure.