diff --git a/src/test/app/ExportResultBuilder_test.cpp b/src/test/app/ExportResultBuilder_test.cpp index c88944b82..9cace80fb 100644 --- a/src/test/app/ExportResultBuilder_test.cpp +++ b/src/test/app/ExportResultBuilder_test.cpp @@ -332,6 +332,34 @@ public: } } + void + testRejectsNonCanonicalWitnessSigner() + { + testcase("signature witness validates signer account"); + + auto const src = randomKeyPair(KeyType::secp256k1); + auto const dst = randomKeyPair(KeyType::secp256k1); + auto const signer = randomKeyPair(KeyType::secp256k1); + auto const wrongAccount = randomKeyPair(KeyType::secp256k1); + auto const innerTx = makeExportedPayment( + calcAccountID(src.first), calcAccountID(dst.first)); + auto const exportTxHash = makeHash("bad-witness-signer"); + + ExportResultBuilder::SignatureSnapshot signatures; + signatures.emplace( + signer.first, + ExportResultBuilder::signExportedTxn( + innerTx, signer.first, signer.second)); + + auto witness = ExportResultBuilder::buildSignatureWitness( + exportTxHash, signatures, 654); + auto signers = witness.getFieldArray(sfSigners); + signers[0].setAccountID(sfAccount, calcAccountID(wrongAccount.first)); + witness.setFieldArray(sfSigners, signers); + + BEAST_EXPECT(!ExportResultBuilder::signaturesFromWitness(witness)); + } + void run() override { @@ -341,6 +369,7 @@ public: testCapsSignerArray(); testAssemblesWitnessReferenceMetadata(); testSignatureWitnessRoundTrip(); + testRejectsNonCanonicalWitnessSigner(); } }; diff --git a/src/test/app/Export_test.cpp b/src/test/app/Export_test.cpp index c9b873ace..065b53c35 100644 --- a/src/test/app/Export_test.cpp +++ b/src/test/app/Export_test.cpp @@ -655,6 +655,33 @@ struct Export_test : public beast::unit_test::suite BEAST_EXPECT(dirIsEmpty(*env.current(), emittedDirKey)); } + void + testExportRejectsAmbiguousLowNetworkID() + { + testcase("Export rejects absent NetworkID on low source networks"); + + using namespace jtx; + + Account const alice{"alice"}; + Account const carol{"carol"}; + auto innerObj = buildExportedPayment(alice.id(), carol.id(), 2, 6); + auto const innerTx = makeSTTx(innerObj); + auto const j = beast::Journal{beast::Journal::getNullSink()}; + + // Low-ID networks do not encode sfNetworkID on ordinary transactions, + // so an absent destination ID is ambiguous with a self-target. + BEAST_EXPECT( + ExportLedgerOps::validateNetworkID(innerTx, 0, j) == temMALFORMED); + BEAST_EXPECT( + ExportLedgerOps::validateNetworkID(innerTx, 1024, j) == + temMALFORMED); + + // A high-ID source chain can still export to a legacy low-ID + // destination whose transactions omit sfNetworkID. + BEAST_EXPECT( + ExportLedgerOps::validateNetworkID(innerTx, 1025, j) == tesSUCCESS); + } + void testXportEmissionLimit(FeatureBitset features) { @@ -868,7 +895,8 @@ struct Export_test : public beast::unit_test::suite expectedSigs.emplace(valPK, originalSig); auto const exportSignatureWitnesses = makeExportSignatureWitnesses(txHash, expectedSigs, applySeq); - ApplyOptions const applyOptions{&exportSignatureWitnesses}; + ApplyOptions const applyOptions{ + &exportSignatureWitnesses, false, nullptr}; auto const expectedSignedTxHash = ExportResultBuilder::assemble( innerTx, expectedSigs, applySeq, txHash) @@ -938,7 +966,8 @@ struct Export_test : public beast::unit_test::suite auto const exportSignatureWitnesses = makeExportSignatureWitnesses(txHash, signatures, applySeq); - ApplyOptions const liveOptions{&exportSignatureWitnesses}; + ApplyOptions const liveOptions{ + &exportSignatureWitnesses, false, nullptr}; { auto next = std::make_shared( *parent, env.app().timeKeeper().closeTime()); @@ -952,7 +981,8 @@ struct Export_test : public beast::unit_test::suite // Historical replay reconstructs an already-validated ledger. The // persisted witness supplies the historical signing-key membership; // current ManifestCache may no longer know a rotated signing key. - ApplyOptions const replayOptions{&exportSignatureWitnesses, true}; + ApplyOptions const replayOptions{ + &exportSignatureWitnesses, true, nullptr}; auto const expectedSignedTxHash = ExportResultBuilder::assemble(innerTx, signatures, applySeq, txHash) .signedTxHash; @@ -1104,7 +1134,8 @@ struct Export_test : public beast::unit_test::suite exportSignatureWitnesses = makeExportSignatureWitnesses(txHash, signatures, applySeq); } - ApplyOptions const applyOptions{&exportSignatureWitnesses}; + ApplyOptions const applyOptions{ + &exportSignatureWitnesses, false, nullptr}; auto next = std::make_shared( *parent, env.app().timeKeeper().closeTime()); @@ -1645,6 +1676,7 @@ struct Export_test : public beast::unit_test::suite testXportPayment(allWithExport); testXportRejectsLocalNetworkID(allWithExport); testXportRejectsUnconfiguredNetworkID(allWithExport); + testExportRejectsAmbiguousLowNetworkID(); testXportEmissionLimit(allWithExport); // ttEXPORT transactor tests diff --git a/src/xrpld/app/consensus/ExportIntent.md b/src/xrpld/app/consensus/ExportIntent.md index af01e5164..6f1724988 100644 --- a/src/xrpld/app/consensus/ExportIntent.md +++ b/src/xrpld/app/consensus/ExportIntent.md @@ -101,7 +101,9 @@ must follow the same deterministic contract as `ExportResultBuilder`: sort signers canonically by AccountID, use an empty `SigningPubKey`, and cap the target-chain `Signers` array at `STTx::maxMultiSigners()` before computing or submitting the blob. The witness may contain extra source-side signatures that -are valid replay input but are not part of the target-chain blob. +are valid replay input but are not part of the target-chain blob. Source-chain +export does not prove the destination account's SignerList or quorum policy; +that compatibility is an operator/client contract for the chosen target chain. This is not an XPOP-style self-contained proof. XPOP embeds its UNL and manifest bundle because it is imported as external proof material. Export witnesses are diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 8037d6ca6..485d0b671 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -1010,6 +1010,7 @@ RCLConsensus::Adaptor::onModeChange(ConsensusMode before, ConsensusMode after) ConsensusPhase RCLConsensus::phase() const { + std::lock_guard _{mutex_}; return consensus_->phase(); } diff --git a/src/xrpld/app/ledger/detail/BuildLedger.cpp b/src/xrpld/app/ledger/detail/BuildLedger.cpp index 26311d4ed..647cdba05 100644 --- a/src/xrpld/app/ledger/detail/BuildLedger.cpp +++ b/src/xrpld/app/ledger/detail/BuildLedger.cpp @@ -68,7 +68,9 @@ collectExportSignatureWitness( if (tx.getFieldU32(sfLedgerSequence) != ledgerSeq || tx.getAccountID(sfAccount) != AccountID{} || tx.getFieldU32(sfSequence) != 0 || - tx.getFieldAmount(sfFee) != beast::zero) + tx.getFieldAmount(sfFee) != beast::zero || + !tx.getSigningPubKey().empty() || !tx.getSignature().empty() || + tx.isFieldPresent(sfPreviousTxnID)) { JLOG(j.warn()) << "Export: ignoring non-canonical signature witness" << " witnessHash=" << tx.getTransactionID() @@ -203,7 +205,7 @@ applyTransactions( view.seq(), j); } - ApplyOptions const applyOptions{&exportSignatureWitnesses}; + ApplyOptions const applyOptions{&exportSignatureWitnesses, false, nullptr}; //@@start rng-entropy-first-application // CRITICAL: Apply consensus entropy pseudo-tx FIRST before any other @@ -393,7 +395,8 @@ buildLedger( accum.seq(), j); } - ApplyOptions const applyOptions{&exportSignatureWitnesses, true}; + ApplyOptions const applyOptions{ + &exportSignatureWitnesses, true, replayData.parent()}; for (auto& tx : replayData.orderedTxns()) applyTransaction( diff --git a/src/xrpld/app/tx/applySteps.h b/src/xrpld/app/tx/applySteps.h index e9f97c23c..d24dd9d1b 100644 --- a/src/xrpld/app/tx/applySteps.h +++ b/src/xrpld/app/tx/applySteps.h @@ -24,9 +24,12 @@ #include #include +#include + namespace ripple { class Application; +class Ledger; class STTx; class TxQ; @@ -56,6 +59,11 @@ struct ApplyOptions // historical manifest map; current ManifestCache state may have rotated // since the ledger closed. bool historicalLedgerReplay = false; + + // LedgerReplay can build consecutive ledgers before the rebuilt parent is + // visible through LedgerMaster. Export apply needs the exact replay parent + // to rebuild the historical validator view. + std::shared_ptr replayParentLedger; }; /** Return true if the transaction can claim a fee (tec), diff --git a/src/xrpld/app/tx/detail/ApplyContext.cpp b/src/xrpld/app/tx/detail/ApplyContext.cpp index 5de9cb456..1ce5a0c79 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.cpp +++ b/src/xrpld/app/tx/detail/ApplyContext.cpp @@ -38,7 +38,8 @@ ApplyContext::ApplyContext( ApplyFlags flags, beast::Journal journal_, ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses, - bool historicalLedgerReplay) + bool historicalLedgerReplay, + std::shared_ptr replayParentLedger) : app(app_) , tx(tx_) , preclaimResult(preclaimResult_) @@ -48,6 +49,7 @@ ApplyContext::ApplyContext( , flags_(flags) , exportSignatureWitnesses_(exportSignatureWitnesses) , historicalLedgerReplay_(historicalLedgerReplay) + , replayParentLedger_(std::move(replayParentLedger)) { view_.emplace(&base_, flags_); } diff --git a/src/xrpld/app/tx/detail/ApplyContext.h b/src/xrpld/app/tx/detail/ApplyContext.h index 7ff33e463..8336119ef 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.h +++ b/src/xrpld/app/tx/detail/ApplyContext.h @@ -27,11 +27,14 @@ #include #include #include +#include #include #include namespace ripple { +class Ledger; + /** State information when applying a tx. */ class ApplyContext { @@ -46,7 +49,8 @@ public: beast::Journal = beast::Journal{beast::Journal::getNullSink()}, ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses = nullptr, - bool historicalLedgerReplay = false); + bool historicalLedgerReplay = false, + std::shared_ptr replayParentLedger = nullptr); Application& app; STTx const& tx; @@ -152,6 +156,12 @@ public: return historicalLedgerReplay_; } + std::shared_ptr + replayParentLedger() const + { + return replayParentLedger_; + } + ApplyFlags const& flags() { @@ -174,6 +184,7 @@ private: std::optional view_; ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses_; bool historicalLedgerReplay_; + std::shared_ptr replayParentLedger_; }; } // namespace ripple diff --git a/src/xrpld/app/tx/detail/Export.cpp b/src/xrpld/app/tx/detail/Export.cpp index 41f5390c9..39135eba8 100644 --- a/src/xrpld/app/tx/detail/Export.cpp +++ b/src/xrpld/app/tx/detail/Export.cpp @@ -125,8 +125,18 @@ Export::doApply() auto& consensusExtensions = ctx_.app.getConsensusExtensions(); bool const standalone = ctx_.app.config().standalone(); auto const& valKeys = ctx_.app.getValidatorKeys(); - auto const parentLedger = - ctx_.app.getLedgerMaster().getLedgerByHash(view().info().parentHash); + auto parentLedger = ctx_.replayParentLedger(); + if (parentLedger && parentLedger->info().hash != view().info().parentHash) + { + JLOG(j_.warn()) << "Export: ignoring mismatched replay parent" + << " txHash=" << txId << " ledgerSeq=" << currentSeq + << " parentHash=" << view().info().parentHash + << " replayParentHash=" << parentLedger->info().hash; + parentLedger.reset(); + } + if (!parentLedger) + parentLedger = ctx_.app.getLedgerMaster().getLedgerByHash( + view().info().parentHash); if (!standalone && !parentLedger) { JLOG(j_.warn()) << "Export: retrying without parent ledger" diff --git a/src/xrpld/app/tx/detail/ExportLedgerOps.h b/src/xrpld/app/tx/detail/ExportLedgerOps.h index b6eff353f..c11cdd346 100644 --- a/src/xrpld/app/tx/detail/ExportLedgerOps.h +++ b/src/xrpld/app/tx/detail/ExportLedgerOps.h @@ -103,8 +103,8 @@ checkExportTxnLimit(ReadView const& view, beast::Journal j) /// - Networks > 1024: sfNetworkID is REQUIRED and must match /// /// So: if exported tx has sfNetworkID matching local → self-target. -/// if local NETWORK_ID is 0 (unconfigured) and tx has no -/// sfNetworkID → can't distinguish self from cross-chain, reject. +/// if local NETWORK_ID <= 1024 and tx has no sfNetworkID → can't +/// distinguish self from another low-ID chain, reject. inline TER validateNetworkID( STTx const& stx, @@ -120,10 +120,10 @@ validateNetworkID( return temMALFORMED; } - if (localNetworkID == 0 && !stx.isFieldPresent(sfNetworkID)) + if (localNetworkID <= 1024 && !stx.isFieldPresent(sfNetworkID)) { JLOG(j.warn()) << "ExportLedgerOps: rejected export with " - "unconfigured NETWORK_ID"; + "ambiguous low NETWORK_ID"; return temMALFORMED; } diff --git a/src/xrpld/app/tx/detail/ExportResultBuilder.cpp b/src/xrpld/app/tx/detail/ExportResultBuilder.cpp index 6591ee668..5e5d79e87 100644 --- a/src/xrpld/app/tx/detail/ExportResultBuilder.cpp +++ b/src/xrpld/app/tx/detail/ExportResultBuilder.cpp @@ -40,7 +40,8 @@ buildSigners(SignatureSnapshot const& signatures, bool capForTargetChain) // XRPL validates the Signers array size before checking signer weights. // Export quorum is decided earlier from the agreed sidecar snapshot; this - // cap only materializes a target-chain-valid canonical prefix. + // cap only materializes a protocol-size-valid canonical prefix. The + // destination account's SignerList/quorum remains an operator contract. if (capForTargetChain) { auto const maxSigners = STTx::maxMultiSigners(); @@ -114,6 +115,7 @@ signaturesFromWitness(STTx const& witness) for (auto const& signer : witness.getFieldArray(sfSigners)) { if (signer.getFName() != sfSigner || + !signer.isFieldPresent(sfAccount) || !signer.isFieldPresent(sfSigningPubKey) || !signer.isFieldPresent(sfTxnSignature)) return std::nullopt; @@ -122,6 +124,10 @@ signaturesFromWitness(STTx const& witness) if (!publicKeyType(makeSlice(pkBlob))) return std::nullopt; + if (signer.getAccountID(sfAccount) != + calcAccountID(PublicKey(makeSlice(pkBlob)))) + return std::nullopt; + auto const sigBlob = signer.getFieldVL(sfTxnSignature); if (sigBlob.empty() || sigBlob.size() > maxTxnSignatureBytes) return std::nullopt; diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index d1def68d6..2e091b93f 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -411,7 +411,8 @@ doApply( preclaimResult.flags, preclaimResult.j, options.exportSignatureWitnesses, - options.historicalLedgerReplay); + options.historicalLedgerReplay, + options.replayParentLedger); return invoke_apply(ctx); } catch (std::exception const& e)