GenerateOrder can be replayed due to constant nonce

[hats-bug-reporter[bot]]

Github username: @SB-Security Twitter username: SBSecurity_ Submission hash (on-chain): 0x062f40be67b52de94896f2ff5558a062a4cf998b0a315ce87b133edd596db1a7 Severity: medium

Description: Description\ VaultBitcoinWallet::generateOrder calls _encryptPayload which is not updating the nonce and due to the arbitrary arguments passed orders can be replayed.

Attack Scenario\ UserSeed can be derived again as it is not random. Then this seed will be used to encode recoveryData and all parameters in encodedData can be passed the same. Then inside _encryptPayload it uses the same nonce every time. Which always _encryptedOrder to be replayed. Also the _offchainPubKeyIndex is not being altered in none of the flow which is also why tx will be able to be replayed.

Attachments

1. Proof of Concept (PoC) File

function generateOrder(
        address to,
        bytes memory _data,
        bytes32 _entropy
    ) public view returns (bytes memory orderData, bytes memory btcAddr) {
        uint256 _keyIndex = _ringKeys.length - 1;
        uint256 _offchainPubKeyIndex = offchainSignerPubKeys.length - 1;

        bytes32 userSeed = _random(_entropy);
        bytes memory recoveryData = abi.encode(_offchainPubKeyIndex, userSeed, to, _data);

        bytes20 _scriptHash = _keyDataToScriptHash(_offchainPubKeyIndex, _keyIndex, keccak256(recoveryData));

        (bytes memory _encryptedOrder,) = _encryptPayload(recoveryData);

        orderData = abi.encode(_keyIndex, _encryptedOrder);
        btcAddr = _generateAddress(_scriptHash, _isTestnet() ? TYPE_ADDR_P2SH_TESTNET : TYPE_ADDR_P2SH);
    }

 function _computeNonce(uint256 keyIndex) private view returns (bytes32 nonce) {
        nonce = keccak256(abi.encodePacked(keyIndex, nonceConst));
    }

    function _encryptPayload(bytes memory payload) internal view returns (bytes memory encryptedData, uint256 keyIndex) {
        require(_ringKeys.length > 0, "No ring keys set up");

        keyIndex = _ringKeys.length - 1;
        bytes32 nonce = _computeNonce(keyIndex);
        encryptedData = Sapphire.encrypt(_ringKeys[keyIndex], bytes32(nonce), payload, abi.encodePacked(nonceConst));
    }

1. Revised Code File (Optional)

Make the nonce incrementalble.

[party-for-illuminati]

That's how it should work, same payload must result to the same deposit address