Summary

Both the _refundETH and _refundERC20 functions have the same issue with the order of state changes and transfers. In both functions, the contract first performs the transfer and then resets the user's balance. It is generally safer and more consistent to reset the user's balance before the transfer, even if reentrancy is not a concern.

Vulnerability Details

• In the _refundETH function, Ether is transferred to the user before the user's balance is reset. If the balance is not cleared first, the state could be inconsistent, especially if the contract logic changes in the future.

• In the _refundERC20 function, token transfers are executed before the balances are reset, which could also lead to inconsistent states or unintended behaviors, even though reentrancy attacks are not a concern here.

• While there are no immediate issues with reentrancy for external owned accounts (EOAs), resetting the balance before performing the transfer is a best practice for consistency and security.

Impact

Though reentrancy is not a concern in this specific case (assuming the recipient is an EOA), this ordering can lead to inconsistent states, especially if the code is modified in the future. The user's balance should be cleared before any transfers occur to maintain a predictable and secure contract state.

Tools Used

• Manual review

Recommendations

To follow best practices, it is recommended to reset the balances before performing the transfers. The corrected functions should look like this: