fix: Give a multi-signed BatchSigner its own signing prefix

A batch signer signs one of two payloads: the batch data plus its own
account, or, when it signs from a signer list, the batch data plus its
own account plus the signer's account. Both used the BCH prefix, so the
two payloads shared a hash space, unlike TxSign and TxMultiSign, which
are split for exactly this reason.

The multi-signing form now uses a BCM prefix. The two payloads already
differed in length, so this closes no attack; it applies the convention
that every distinct signing payload gets its own prefix.

Batch is gated on featureBatchV1_1, which is not enabled on any network,
so no existing signature changes and no amendment gate is needed.

API-CHANGELOG.md

@@ -41,6 +41,7 @@ This section contains changes targeting a future version.
 
 ### Bugfixes
 
+- `sign_for`, `submit_multisigned`: A `BatchSigner` that signs from a signer list now covers a different signing prefix (`BCM`) than a `BatchSigner` that signs on its own (`BCH`), matching the existing split between `STX` and `SMT`. Clients that build batch signatures themselves must use the new prefix for the multi-signing form. `Batch` is not enabled on any network, so no existing signature is affected.
 - `get_aggregate_price`: Duplicate entries in the `oracles` request array are now ignored. [#6586](https://github.com/XRPLF/rippled/pull/6586)
 - Peer Crawler: The `port` field in `overlay.active[]` now consistently returns an integer instead of a string for outbound peers. [#6318](https://github.com/XRPLF/rippled/pull/6318)
 - `ping`: The `ip` field is no longer returned as an empty string for proxied connections without a forwarded-for header. It is now omitted, consistent with the behavior for identified connections. [#6730](https://github.com/XRPLF/rippled/pull/6730)

include/xrpl/protocol/Batch.h

@@ -10,15 +10,24 @@
 
 namespace xrpl {
 
+/**
+ * Serialize the data that a batch signer signs.
+ *
+ * @param prefix HashPrefix::Batch when the batch signer signs on its own,
+ * HashPrefix::BatchMultiSign when the signature comes from a signer list. The
+ * two forms differ by the signer account that the caller appends, so the
+ * prefix keeps them in separate hash spaces, as with TxSign and TxMultiSign.
+ */
 inline void
 serializeBatch(
     Serializer& msg,
+    HashPrefix prefix,
     AccountID const& outerAccount,
     std::uint32_t outerSeqValue,
     std::uint32_t const& flags,
     std::vector<uint256> const& txids)
 {
-    msg.add32(HashPrefix::Batch);
+    msg.add32(prefix);
     msg.addBitString(outerAccount);
     msg.add32(outerSeqValue);
     msg.add32(flags);

include/xrpl/protocol/HashPrefix.h

@@ -89,9 +89,14 @@ enum class HashPrefix : std::uint32_t {
     PaymentChannelClaim = detail::makeHashPrefix('C', 'L', 'M'),
 
     /**
-     * Batch
+     * Batch to sign
      */
     Batch = detail::makeHashPrefix('B', 'C', 'H'),
+
+    /**
+     * Batch to multi-sign
+     */
+    BatchMultiSign = detail::makeHashPrefix('B', 'C', 'M'),
 };
 
 template <class Hasher>

src/libxrpl/protocol/STTx.cpp

@@ -458,7 +458,8 @@ STTx::checkBatchSingleSign(STObject const& batchSigner, std::vector<uint256> con
 {
     XRPL_ASSERT(getTxnType() == ttBATCH, "STTx::checkBatchSingleSign : batch transaction");
     Serializer msg;
-    serializeBatch(msg, getAccountID(sfAccount), getSeqProxy().value(), getFlags(), txIds);
+    serializeBatch(
+        msg, HashPrefix::Batch, getAccountID(sfAccount), getSeqProxy().value(), getFlags(), txIds);
     finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg);
     return singleSignHelper(batchSigner, msg.slice());
 }
@@ -552,7 +553,13 @@ STTx::checkBatchMultiSign(
     // with the stuff that stays constant from signature to signature.
     auto const batchSignerAccount = batchSigner.getAccountID(sfAccount);
     Serializer dataStart;
-    serializeBatch(dataStart, getAccountID(sfAccount), getSeqProxy().value(), getFlags(), txIds);
+    serializeBatch(
+        dataStart,
+        HashPrefix::BatchMultiSign,
+        getAccountID(sfAccount),
+        getSeqProxy().value(),
+        getFlags(),
+        txIds);
     dataStart.addBitString(batchSignerAccount);
     return multiSignHelper(
         batchSigner,

src/test/app/Batch_test.cpp

@@ -31,6 +31,7 @@
 #include <xrpld/app/misc/TxQ.h>
 
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/safe_cast.h>
 #include <xrpl/basics/strHex.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/beast/utility/Journal.h>
@@ -648,6 +649,7 @@ class Batch_test : public beast::unit_test::Suite
             Serializer msg;
             serializeBatch(
                 msg,
+                HashPrefix::Batch,
                 jt.stx->getAccountID(sfAccount),
                 jt.stx->getSeqProxy().value(),
                 tfAllOrNothing,
@@ -2910,6 +2912,79 @@ class Batch_test : public beast::unit_test::Suite
         env.close();
     }
 
+    void
+    testSigningPrefixes(FeatureBitset features)
+    {
+        testcase("signing prefixes");
+
+        using namespace test::jtx;
+        using namespace std::literals;
+
+        // A batch signer signs one of two payloads: the batch data plus its
+        // own account, or, when it signs from a signer list, the batch data
+        // plus its own account plus the signer's account. Each payload has its
+        // own prefix, so a signature made for one cannot be read as the other.
+        static_assert(safeCast<std::uint32_t>(HashPrefix::Batch) == 0x42434800);
+        static_assert(safeCast<std::uint32_t>(HashPrefix::BatchMultiSign) == 0x42434D00);
+
+        Env env{*this, features};
+
+        auto const alice = Account("alice");
+        auto const bob = Account("bob");
+        auto const carol = Account("carol");
+        auto const dave = Account("dave");
+        env.fund(XRP(10000), alice, bob, carol, dave);
+        env.close();
+
+        env(signers(bob, 1, {{carol, 1}, {dave, 1}}));
+        env.close();
+
+        // A multi-signature over the single-signing payload is rejected.
+        {
+            auto const seq = env.seq(alice);
+            auto const batchFee = batch::calcBatchFee(env, 2, 2);
+            auto jt = env.jtnofill(
+                batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::Inner(pay(alice, bob, XRP(10)), seq + 1),
+                batch::Inner(pay(bob, alice, XRP(5)), env.seq(bob)));
+
+            Serializer msg;
+            serializeBatch(
+                msg,
+                HashPrefix::Batch,
+                jt.stx->getAccountID(sfAccount),
+                jt.stx->getSeqProxy().value(),
+                tfAllOrNothing,
+                jt.stx->getBatchTransactionIDs());
+            msg.addBitString(bob.id());
+            finishMultiSigningData(carol.id(), msg);
+            auto const sig = xrpl::sign(carol.pk(), carol.sk(), msg.slice());
+
+            auto& bso = jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName];
+            bso[sfAccount.jsonName] = bob.human();
+            bso[sfSigningPubKey.jsonName] = "";
+            auto& iso = bso[sfSigners.jsonName][0u][sfSigner.jsonName];
+            iso[sfAccount.jsonName] = carol.human();
+            iso[sfSigningPubKey.jsonName] = strHex(carol.pk());
+            iso[sfTxnSignature.jsonName] = strHex(Slice{sig.data(), sig.size()});
+
+            env(jt.jv, Ter(telENV_RPC_FAILED));
+            env.close();
+        }
+
+        // The same signature over the multi-signing payload is accepted.
+        {
+            auto const seq = env.seq(alice);
+            auto const batchFee = batch::calcBatchFee(env, 2, 2);
+            env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
+                batch::Inner(pay(alice, bob, XRP(10)), seq + 1),
+                batch::Inner(pay(bob, alice, XRP(5)), env.seq(bob)),
+                batch::Msig(bob, {carol}),
+                Ter(tesSUCCESS));
+            env.close();
+        }
+    }
+
     void
     testAccountSet(FeatureBitset features)
     {
@@ -5913,6 +5988,7 @@ class Batch_test : public beast::unit_test::Suite
         testInnerSubmitRPC(features);
         testAccountActivation(features);
         testCheckAllSignatures(features);
+        testSigningPrefixes(features);
         testAccountSet(features);
         testAccountDelete(features);
         testLoan(features);

src/test/jtx/impl/batch.cpp

@@ -101,6 +101,7 @@ Sig::operator()(Env& env, JTx& jt) const
         Serializer msg;
         serializeBatch(
             msg,
+            HashPrefix::Batch,
             stx.getAccountID(sfAccount),
             stx.getSeqProxy().value(),
             stx.getFlags(),
@@ -145,6 +146,7 @@ Msig::operator()(Env& env, JTx& jt) const
         Serializer msg;
         serializeBatch(
             msg,
+            HashPrefix::BatchMultiSign,
             stx.getAccountID(sfAccount),
             stx.getSeqProxy().value(),
             stx.getFlags(),