hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

1 stars 0 forks source link

Use `forceApprove` instead of `approve` in `Router::_splitPosition` #43

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

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

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xea7ee0dc91116cf3a59957c1a455c818a86977ff22277db7283334b6bb3f5348 Severity: medium

Description: Description

Some tokens such as USDT in mainnet will check for allowance is equal to zero in approve function, if the allowance is not zero, the approve function will revert in such tokens, therefore it is recommended to use forceApprove from Openzeppelin to approve allowance.

Attack Scenario

Denial of service.

Attachments

NA

  1. Proof of Concept (PoC) File

In Router::_splitPosition:

 function _splitPosition(IERC20 collateralToken, Market market, uint256 amount) internal {
        bytes32 parentCollectionId = market.parentCollectionId();
        bytes32 conditionId = market.conditionId();

        uint256[] memory partition = getPartition(conditionalTokens.getOutcomeSlotCount(conditionId));

        if (parentCollectionId != bytes32(0)) {
            // it's splitting from a parent position, so we need to unwrap these tokens first because they will be burnt to mint the child outcome tokens.
            (IERC20 wrapped1155, bytes memory data) = market.parentWrappedOutcome();

            uint256 tokenId = conditionalTokens.getPositionId(address(collateralToken), parentCollectionId);

            wrapped1155.transferFrom(msg.sender, address(this), amount);
            wrapped1155Factory.unwrap(address(conditionalTokens), tokenId, amount, address(this), data);
        } else {
            collateralToken.approve(address(conditionalTokens), amount); //@audit Should use forceApprove here.
        }

        conditionalTokens.splitPosition(address(collateralToken), parentCollectionId, conditionId, partition, amount);

        // wrap & transfer the minted outcome tokens.
        for (uint256 j = 0; j < partition.length; j++) {
            uint256 tokenId = getTokenId(collateralToken, parentCollectionId, conditionId, partition[j]);

            (IERC20 wrapped1155, bytes memory data) = market.wrappedOutcome(j);

            // wrap to erc20.
            conditionalTokens.safeTransferFrom(address(this), address(wrapped1155Factory), tokenId, amount, data);

            // transfer the ERC20 back to the user.
            wrapped1155.transfer(msg.sender, amount);
        }
    }
  1. Revised Code File (Optional)

    use forceApprove from Openzeppelin instead of approve.

clesaege commented 1 month ago

That's a bug in USD.

(For the history, I'm actually the one who fixed an issue in the ERC20 standard where it was showing a token with this bug as an example)

As per context rules, are excluded: