hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Router.sol is not using safeTransfer for token transfer #32

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9f21743900eca60d55c422c10aa3938f51e21868e1eb01d4cdd4705f55c0f569 Severity: medium

Description: Description\ Router.sol is not using safeTransfer for token transfer

Attack Scenario\ _splitPosition function Splits a position and sends the ERC20 outcome tokens to the user, however while doing so it uses transfer instead of safeTransfer which could be problematic Attachments

  1. Proof of Concept (PoC) File It is a good idea to add a require() statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer()/safeTransferFrom() unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

However, using require() to check transfer return values could lead to issues with non-compliant ERC20 tokens which do not return a boolean value. Therefore, it's highly advised to use OpenZeppelin’s safeTransfer()/safeTransferFrom().

  1. Revised Code File (Optional) Consider using safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom().
greenlucid commented 1 month ago

ERC20 does not have safeTransfer

clesaege commented 1 month ago

Similar to https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/issues/16