Description

During the check of a token set by the function checkTokenset, the chain string from the Token structure is compared with the chain from the swap:

function checkTokenset(Token[] memory tokenset, string[] memory addressList) internal view {
    require(tokenset.length == addressList.length, "tokenset length not match addressList length");
    for (uint i = 0; i < tokenset.length; i++) {
        require(bytes32(bytes(tokenset[i].chain)) == bytes32(bytes(chain)), "chain not match");
        address tokenAddress = Utils.stringToAddress(tokenset[i].addr);
        require(tokenAddress != address(0), "zero token address");
        address receiveAddress = Utils.stringToAddress(addressList[i]);
        require(receiveAddress != address(0), "zero receive address");
    }
}

The casting of bytes32(bytes(chain)) truncates the string to exactly 32 bytes, and the comparison is done only on the first 32 bytes of the chain string, further bytes will be skipped. This check is also implemented in the addMintRequest, rejectMintRequest, confirmMintRequest, addRedeemRequest, and addBurnFeeRequest functions.

Impact

In the case that the chains have more than 32 bytes, some tokens with different chains, but a common 32-byte prefix, would be considered equal by the check, allowing them in the set even though they should not.

Recommendations

It would be less error-prone to declare the chain as a bytes32 instead of a string. It would also save gas by removing the comparisons, which are currently made with the Keccak hash function, like in the checkHedgeOrder function of the USSI contract:

function checkHedgeOrder(HedgeOrder calldata hedgeOrder, bytes32 orderHash, bytes calldata orderSignature) public view {
    require(keccak256(abi.encode(chain)) == keccak256(abi.encode(hedgeOrder.chain)), "chain not match");
    //...

Remediation

This issue has been acknowledged by SoSoValue, and a fix was implemented in commit 2cc062d0↗.