Potential Fund Lock Due to Blacklisted Accounts in Router Contract

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

Description:

Description

The Router contract is designed to work with any ERC20 token as collateral for conditional token operations. According to the NATSPEC comments, the contract "replicates the main Conditional Tokens functions, but allowing to work with ERC20 outcomes instead of the ERC1155." This flexibility, while powerful, introduces a potential vulnerability when using centralized stablecoins like USDC or USDT as collateral.

Key functions such as splitPosition, mergePositions, and redeemPositions interact with the collateral token using standard ERC20 methods.

The contract assumes these operations will always succeed for any ERC20 token. However, centralized stablecoins like USDC and USDT have the ability to blacklist addresses, which can cause transfers to or from these addresses to fail.

If a user's address becomes blacklisted by a centralized stablecoin issuer:

Users may be unable to split positions, as the transferFrom operation in splitPosition would fail.
The contract may be unable to return collateral to users in mergePositions or redeemPositions, effectively locking user funds.
Unwrapping operations in _mergePositions and _redeemPositions could fail if they involve transferring blacklisted tokens.

Proof of Concept (PoC) File

Problem areas in the Router.sol contract:

a) splitPosition function:

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/main/contracts/src/Router.sol#L37-L37

function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public {
    if (market.parentCollectionId() == bytes32(0)) {
        // This transfer could fail if the user is blacklisted
        collateralToken.transferFrom(msg.sender, address(this), amount);
    }
    _splitPosition(collateralToken, market, amount);
}

b) mergePositions function:

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/main/contracts/src/Router.sol#L90-L90

function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {
    _mergePositions(collateralToken, market, amount);

    if (market.parentCollectionId() == bytes32(0)) {
        // This transfer could fail if the contract is blacklisted
        collateralToken.transfer(msg.sender, amount);
    }
}

c) redeemPositions function:

https://github.com/hats-finance/SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7/blob/main/contracts/src/Router.sol#L143-L143

function redeemPositions(IERC20 collateralToken, Market market, uint256[] calldata outcomeIndexes) public {
    // ... (other function code)

    if (parentCollectionId == bytes32(0)) {
        uint256 finalBalance = collateralToken.balanceOf(address(this));

        if (finalBalance > initialBalance) {
            // This transfer could fail if the user is blacklisted
            collateralToken.transfer(msg.sender, finalBalance - initialBalance);
        }
    }
}

Recommended Mitigations

Implement transfer result checking: For all ERC20 token transfers in the contract, add a check for the success of the transfer. This can be done by capturing the return value of the transfer and transferFrom functions and reverting the transaction if the transfer fails.

Example:
```
bool success = collateralToken.transfer(recipient, amount);
require(success, "Token transfer failed");
```

This provides an approach to addressing the issue without overly complicating the contract.

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7