search

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

wrapped1155 contract should check return value of transferFrom() method #126

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

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

Github username: @cpp-phoenix Twitter username: 0xrochimaru Submission hash (on-chain): 0xf427b6386fa33a850e1cb250bca81406c37895c18fecc0689002f451b980e5b8 Severity: high

Description: Description\ The contract wrapper1155 return bool on transferFrom() call. Which is not checked. It is assumed to be true.

Attack Scenario\ There can be a scenario if the transferFrom() fails, it returns the bool false instead of reverting. Which will lead to 2 exploits.

  1. In _mergePositions() users will be able to get CollateralTokens without providing any WrappedERC1155 tokens. https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/6e5db716e44e251fcee6abd7c1f6a8d6e36db910/contracts/src/Router.sol#L117
  2. In redeemPositions any user will be able to claim winning amount. https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/6e5db716e44e251fcee6abd7c1f6a8d6e36db910/contracts/src/Router.sol#L182
Auditor0x18 commented 1 month ago

The wrapper1155 contract inherits the OpenZeppelin's ERC20 contract. When the transferFrom() function fails, it always reverts.

File: contracts\src\interaction\1155-to-20\Wrapped1155Factory.sol
14: import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

31: contract Wrapped1155 is Wrapped1155Metadata, ERC20 {
clesaege commented 1 month ago

This looks like it never returns false. Please provide a test showing it returning false and the impact if you still think the issue is correct.