diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 7af96d3f4b..47c76cecd7 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -181,15 +181,13 @@ Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask) if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatchV1_1)) return temINVALID_FLAG; - if (ctx.rules.enabled(featureBatchV1_1) && - ctx.tx.isFlag(tfInnerBatchTxn) != ctx.parentBatchId.has_value()) + // Reject if the inner batch flag and parentBatchId are inconsistent. + // A standalone tx with tfInnerBatchTxn but no parentBatchId is an + // attack attempt. A tx with parentBatchId but without tfInnerBatchTxn + // is a programming error. + if (ctx.tx.isFlag(tfInnerBatchTxn) != ctx.parentBatchId.has_value()) return temINVALID_INNER_BATCH; - XRPL_ASSERT( - ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() || - !ctx.rules.enabled(featureBatchV1_1), - "Inner batch transaction must have a parent batch ID."); - return tesSUCCESS; } @@ -204,15 +202,18 @@ Transactor::preflight2(PreflightContext const& ctx) return *ret; } - // It should be impossible for the InnerBatchTxn flag to be set without - // featureBatchV1_1 being enabled - XRPL_ASSERT_PARTS( - !ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatchV1_1), - "xrpl::Transactor::preflight2", - "InnerBatch flag only set if feature enabled"); - // Skip signature check on batch inner transactions - if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1)) + // Skip signature check on batch inner transactions, but only if + // this tx was actually submitted through the batch apply path + // (parentBatchId is set). Without this check, a standalone tx + // with tfInnerBatchTxn could bypass signature verification. + if (ctx.tx.isFlag(tfInnerBatchTxn)) + { + if (!ctx.rules.enabled(featureBatchV1_1)) + return temINVALID_FLAG; + if (!ctx.parentBatchId.has_value()) + return temINVALID_INNER_BATCH; return tesSUCCESS; + } // Do not add any checks after this point that are relevant for // batch inner transactions. They will be skipped. diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 143ecaa241..9e35d49fcc 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -4411,6 +4411,53 @@ class Batch_test : public beast::unit_test::suite } } + void + testStandaloneInnerBatchFlag(FeatureBitset features) + { + testcase("standalone tx with tfInnerBatchTxn rejected"); + + using namespace test::jtx; + using namespace std::literals; + + // A standalone Payment with tfInnerBatchTxn must be rejected. + // Without proper guards this would bypass signature verification + // in preflight2. + { + test::jtx::Env env{*this, features}; + + auto const alice = Account("alice"); + auto const bob = Account("bob"); + env.fund(XRP(10000), alice, bob); + env.close(); + + // Submit a normal Payment with tfInnerBatchTxn flag. + // preflight1 must reject with temINVALID_INNER_BATCH because + // the flag is set but no parentBatchId exists. + env(pay(alice, bob, XRP(1)), txflags(tfInnerBatchTxn), ter(telENV_RPC_FAILED)); + env.close(); + + // Verify via direct apply path (bypassing RPC layer) + env.app().openLedger().modify([&](OpenView& view, beast::Journal j) { + // Construct a Payment STTx with tfInnerBatchTxn, + // empty signing pub key, and no signature — mimicking + // what an attacker would send to skip sig verification. + STTx const stx = STTx(ttPAYMENT, [&](auto& obj) { + obj.setAccountID(sfAccount, alice.id()); + obj.setAccountID(sfDestination, bob.id()); + obj.setFieldAmount(sfAmount, XRP(1)); + obj.setFieldAmount(sfFee, XRP(0)); + obj.setFieldU32(sfSequence, env.seq(alice)); + obj.setFieldU32(sfFlags, tfInnerBatchTxn); + }); + + auto const result = xrpl::apply(env.app(), view, stx, tapNONE, j); + // Must NOT be applied — signature was never checked + BEAST_EXPECT(!result.applied); + return false; + }); + } + } + void testWithFeats(FeatureBitset features) { @@ -4445,6 +4492,7 @@ class Batch_test : public beast::unit_test::suite testBatchDelegate(features); testValidateRPCResponse(features); testBatchCalculateBaseFee(features); + testStandaloneInnerBatchFlag(features); } public: