fix: bind batch signer signatures to outer account and sequence

This commit is contained in:
Denis Angell
2026-04-21 07:06:17 +02:00
parent 477532edfe
commit 7618b726b3
4 changed files with 192 additions and 7 deletions

View File

@@ -1,5 +1,6 @@
#pragma once
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/HashPrefix.h>
#include <xrpl/protocol/STVector256.h>
#include <xrpl/protocol/Serializer.h>
@@ -7,9 +8,16 @@
namespace xrpl {
inline void
serializeBatch(Serializer& msg, std::uint32_t const& flags, std::vector<uint256> const& txids)
serializeBatch(
Serializer& msg,
AccountID const& outerAccount,
std::uint32_t outerSeqValue,
std::uint32_t const& flags,
std::vector<uint256> const& txids)
{
msg.add32(HashPrefix::batch);
msg.addBitString(outerAccount);
msg.add32(outerSeqValue);
msg.add32(flags);
msg.add32(std::uint32_t(txids.size()));
for (auto const& txid : txids)

View File

@@ -435,7 +435,12 @@ Expected<void, std::string>
STTx::checkBatchSingleSign(STObject const& batchSigner) const
{
Serializer msg;
serializeBatch(msg, getFlags(), getBatchTransactionIDs());
serializeBatch(
msg,
getAccountID(sfAccount),
getSeqValue(),
getFlags(),
getBatchTransactionIDs());
finishMultiSigningData(batchSigner.getAccountID(sfAccount), msg);
return singleSignHelper(batchSigner, msg.slice());
}
@@ -523,11 +528,18 @@ STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const
// We can ease the computational load inside the loop a bit by
// pre-constructing part of the data that we hash. Fill a Serializer
// with the stuff that stays constant from signature to signature.
auto const batchSignerAccount = batchSigner.getAccountID(sfAccount);
Serializer dataStart;
serializeBatch(dataStart, getFlags(), getBatchTransactionIDs());
serializeBatch(
dataStart,
getAccountID(sfAccount),
getSeqValue(),
getFlags(),
getBatchTransactionIDs());
dataStart.addBitString(batchSignerAccount);
return multiSignHelper(
batchSigner,
std::nullopt,
std::optional<AccountID>(batchSignerAccount),
[&dataStart](AccountID const& accountID) -> Serializer {
Serializer s = dataStart;
finishMultiSigningData(accountID, s);

View File

@@ -570,7 +570,12 @@ class Batch_test : public beast::unit_test::suite
batch::inner(pay(bob, alice, XRP(5)), bobSeq));
Serializer msg;
serializeBatch(msg, tfAllOrNothing, jt.stx->getBatchTransactionIDs());
serializeBatch(
msg,
jt.stx->getAccountID(sfAccount),
jt.stx->getSeqValue(),
tfAllOrNothing,
jt.stx->getBatchTransactionIDs());
finishMultiSigningData(bob.id(), msg);
auto const sig = xrpl::sign(bob.pk(), bob.sk(), msg.slice());
jt.jv[sfBatchSigners.jsonName][0u][sfBatchSigner.jsonName][sfAccount.jsonName] =
@@ -4459,6 +4464,154 @@ class Batch_test : public beast::unit_test::suite
}
}
void
testOuterBinding(FeatureBitset features)
{
testcase("outer binding");
using namespace test::jtx;
// Signatures captured from one outer account cannot be replayed
// under a different outer account.
{
Env env{*this, features};
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
auto const eve = Account("eve");
env.fund(XRP(10000), alice, bob, carol, eve);
env.close();
auto const preCarol = env.balance(carol);
auto const bobSeq = env.seq(bob);
auto const carolSeq = env.seq(carol);
auto const aliceSeq = env.seq(alice);
auto const batchFee1 = batch::calcBatchFee(env, 2, 2);
auto jt1 = env.jt(
batch::outer(alice, aliceSeq, batchFee1, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(100)), bobSeq),
batch::inner(pay(carol, alice, XRP(50)), carolSeq),
batch::sig(bob, carol));
auto const capturedSigners = jt1.jv[sfBatchSigners.jsonName];
env(jt1, ter(tesSUCCESS));
env.close();
BEAST_EXPECT(env.seq(bob) == bobSeq + 1);
BEAST_EXPECT(env.seq(carol) == carolSeq);
auto const batchFee2 = batch::calcBatchFee(env, 2, 2);
auto jt2 = env.jtnofill(
batch::outer(eve, env.seq(eve), batchFee2, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(100)), bobSeq),
batch::inner(pay(carol, alice, XRP(50)), carolSeq));
jt2.jv[sfBatchSigners.jsonName] = capturedSigners;
env(jt2.jv, ter(temBAD_SIGNATURE));
env.close();
BEAST_EXPECT(env.seq(carol) == carolSeq);
BEAST_EXPECT(env.balance(carol) == preCarol);
}
// Signatures are bound to the outer sequence; replaying them
// at a higher sequence must fail.
{
Env env{*this, features};
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
env.fund(XRP(10000), alice, bob, carol);
env.close();
auto const preCarol = env.balance(carol);
auto const bobSeq = env.seq(bob);
auto const carolSeq = env.seq(carol);
auto const aliceSeq1 = env.seq(alice);
auto const batchFee1 = batch::calcBatchFee(env, 2, 2);
auto jt1 = env.jt(
batch::outer(alice, aliceSeq1, batchFee1, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(500)), bobSeq),
batch::inner(pay(carol, alice, XRP(500)), carolSeq),
batch::sig(bob, carol));
auto const capturedSigners = jt1.jv[sfBatchSigners.jsonName];
env(jt1, ter(tesSUCCESS));
env.close();
BEAST_EXPECT(env.seq(bob) == bobSeq + 1);
BEAST_EXPECT(env.seq(carol) == carolSeq);
auto const batchFee2 = batch::calcBatchFee(env, 2, 2);
auto jt2 = env.jtnofill(
batch::outer(alice, env.seq(alice), batchFee2, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(500)), bobSeq),
batch::inner(pay(carol, alice, XRP(500)), carolSeq));
jt2.jv[sfBatchSigners.jsonName] = capturedSigners;
env(jt2.jv, ter(temBAD_SIGNATURE));
env.close();
BEAST_EXPECT(env.balance(carol) == preCarol);
BEAST_EXPECT(env.seq(carol) == carolSeq);
}
// Multi-signed batch signer entries are bound to their account;
// reusing inner signatures under a different batch signer must fail.
{
Env env{*this, features};
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const carol = Account("carol");
auto const dave = Account("dave");
auto const elsa = Account("elsa");
env.fund(XRP(10000), alice, bob, carol, dave, elsa);
env.close();
env(signers(bob, 2, {{dave, 1}, {elsa, 1}}));
env.close();
env(signers(carol, 2, {{dave, 1}, {elsa, 1}}));
env.close();
auto const seq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, 3, 2);
auto jt1 = env.jt(
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, {dave, elsa}),
ter(tesSUCCESS));
auto const bobSignerEntry =
jt1.jv[sfBatchSigners.jsonName][0u];
env(jt1, ter(tesSUCCESS));
env.close();
auto const seq2 = env.seq(alice);
auto const batchFee2 = batch::calcBatchFee(env, 3, 2);
auto jt2 = env.jtnofill(
batch::outer(alice, seq2, batchFee2, tfAllOrNothing),
batch::inner(pay(alice, carol, XRP(10)), seq2 + 1),
batch::inner(pay(carol, alice, XRP(5)), env.seq(carol)));
Json::Value carolSigner;
carolSigner[sfBatchSigner.jsonName][jss::Account] = carol.human();
carolSigner[sfBatchSigner.jsonName][jss::SigningPubKey] = "";
carolSigner[sfBatchSigner.jsonName][sfSigners.jsonName] =
bobSignerEntry[sfBatchSigner.jsonName][sfSigners.jsonName];
jt2.jv[sfBatchSigners.jsonName][0u] = carolSigner;
env(jt2.jv, ter(temBAD_SIGNATURE));
env.close();
}
}
void
testUnsortedBatchSigners(FeatureBitset features)
{
@@ -4529,6 +4682,7 @@ class Batch_test : public beast::unit_test::suite
testValidateRPCResponse(features);
testBatchCalculateBaseFee(features);
testStandaloneInnerBatchFlag(features);
testOuterBinding(features);
testUnsortedBatchSigners(features);
}

View File

@@ -73,7 +73,12 @@ sig::operator()(Env& env, JTx& jt) const
jo[jss::SigningPubKey] = strHex(e.sig.pk().slice());
Serializer msg;
serializeBatch(msg, stx.getFlags(), stx.getBatchTransactionIDs());
serializeBatch(
msg,
stx.getAccountID(sfAccount),
stx.getSeqValue(),
stx.getFlags(),
stx.getBatchTransactionIDs());
finishMultiSigningData(e.acct.id(), msg);
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
auto const sig = xrpl::sign(*publicKeyType(e.sig.pk().slice()), e.sig.sk(), msg.slice());
@@ -112,7 +117,13 @@ msig::operator()(Env& env, JTx& jt) const
iso[jss::SigningPubKey] = strHex(e.sig.pk().slice());
Serializer msg;
serializeBatch(msg, stx.getFlags(), stx.getBatchTransactionIDs());
serializeBatch(
msg,
stx.getAccountID(sfAccount),
stx.getSeqValue(),
stx.getFlags(),
stx.getBatchTransactionIDs());
msg.addBitString(master.id());
finishMultiSigningData(e.acct.id(), msg);
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
auto const sig = xrpl::sign(*publicKeyType(e.sig.pk().slice()), e.sig.sk(), msg.slice());