sherlock-audit / 2023-06-dinari-judging

5 stars 4 forks source link

rugpull_detector - `block.chainid` should be included to generate `orderId` to prevent cross-chain replay attack vector #76

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

rugpull_detector

medium

block.chainid should be included to generate orderId to prevent cross-chain replay attack vector

Summary

getOrderIdFromOrderRequest uses salt, order request, typehash to generate orderId but not block.chainid

Vulnerability Detail

function getOrderIdFromOrderRequest(OrderRequest memory orderRequest, bytes32 salt) public pure returns (bytes32) {
        //@audit - missing block.chainid to generate order id - order id generated for testnet can be replayed for mainnet.
        return keccak256(
            abi.encode(
                ORDERREQUEST_TYPE_HASH,
                salt,
                orderRequest.recipient,
                orderRequest.assetToken,
                orderRequest.paymentToken,
                orderRequest.quantityIn
            )
        );
    }

Impact

Testnet's orderId can be reused in mainnet.

Any future upgrade that relies on orderId might be prone to cross-chain replay attack.

Code Snippet

https://github.com/sherlock-audit/2023-06-dinari/blob/4851cb7ebc86a7bc26b8d0d399a7dd7f9520f393/sbt-contracts/src/issuer/OrderProcessor.sol#L205C1-L216C6

Tool used

Manual Review

Recommendation

Include block.chainid to generate orderId

function getOrderIdFromOrderRequest(OrderRequest memory orderRequest, bytes32 salt) public pure returns (bytes32) {
        //@audit - missing chainid() to generate order id - order id generated for testnet can be replayed for mainnet.
        return keccak256(
            abi.encode(
                ORDERREQUEST_TYPE_HASH,
+              block.chainid,
                salt,
                orderRequest.recipient,
                orderRequest.assetToken,
                orderRequest.paymentToken,
                orderRequest.quantityIn
            )
        );
    }