diff --git a/src/test/app/SetManifest_test.cpp b/src/test/app/SetManifest_test.cpp index 0b10bc4de4..68233bf895 100644 --- a/src/test/app/SetManifest_test.cpp +++ b/src/test/app/SetManifest_test.cpp @@ -956,11 +956,19 @@ struct SetManifest_test : public beast::unit_test::suite auto const priced = envelope(env, good, master.id()); auto const canonicalFee = priced->getFieldAmount(sfFee).xrp(); + auto const nonCanonicalFee = + envelope(env, good, master.id(), [&](STObject& obj) { + obj.setFieldAmount(sfFee, canonicalFee + XRPAmount{1}); + }); + auto const [feeValidity, feeReason] = checkValidity( + env.app().getHashRouter(), + *nonCanonicalFee, + env.current()->rules(), + env.app().config()); + BEAST_EXPECT(feeValidity == Validity::SigBad); BEAST_EXPECT( - applyDirect( - env, envelope(env, good, master.id(), [&](STObject& obj) { - obj.setFieldAmount(sfFee, canonicalFee + XRPAmount{1}); - })) == temBAD_FEE); + feeReason == "Manifest-authorized envelope has non-canonical fee"); + BEAST_EXPECT(applyDirect(env, nonCanonicalFee) == temBAD_FEE); BEAST_EXPECT( applyDirect( diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index d4fcdce5b9..815d3a6112 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -2048,7 +2048,7 @@ TxQ::tryDirectApply( return ApplyResult{pfresult.ter, false}; auto const pcresult = preclaim(pfresult, app, view); - if (!pcresult.likelyToClaimFee) + if (!isTesSuccess(pcresult.ter)) return ApplyResult{pcresult.ter, false}; // A valid canonical update simply rides out the fee storm. Do not diff --git a/src/xrpld/app/tx/detail/SetManifest.cpp b/src/xrpld/app/tx/detail/SetManifest.cpp index ca8895487b..c3ea653958 100644 --- a/src/xrpld/app/tx/detail/SetManifest.cpp +++ b/src/xrpld/app/tx/detail/SetManifest.cpp @@ -122,6 +122,22 @@ hasCanonicalUnsignedSetManifestShape(STTx const& tx) noexcept } } +bool +hasCanonicalUnsignedSetManifestFee(STTx const& tx, Rules const& rules) noexcept +{ + try + { + auto const& manifest = + const_cast(tx).getField(sfManifest).downcast(); + return tx[sfFee].xrp() == + canonicalUnsignedSetManifestFee(rules, manifest); + } + catch (std::exception const&) + { + return false; + } +} + std::optional onLedgerManifestSequence(ReadView const& view, PublicKey const& masterKey) { @@ -164,6 +180,12 @@ SetManifest::preflight(PreflightContext const& ctx) JLOG(j.warn()) << "SetManifest: non-canonical unsigned envelope."; return temMALFORMED; } + if (manifestAuthorityCandidate && + !hasCanonicalUnsignedSetManifestFee(tx, ctx.rules)) + { + JLOG(j.warn()) << "SetManifest: non-canonical unsigned fee."; + return temBAD_FEE; + } bool const manifestAuthorized = isUnsignedSetManifest(tx); // Authenticate the cheapest available outer authority before doing any diff --git a/src/xrpld/app/tx/detail/SetManifest.h b/src/xrpld/app/tx/detail/SetManifest.h index 76facf4165..e60ba54486 100644 --- a/src/xrpld/app/tx/detail/SetManifest.h +++ b/src/xrpld/app/tx/detail/SetManifest.h @@ -57,6 +57,15 @@ isUnsignedSetManifest(STTx const& tx) noexcept; bool hasCanonicalUnsignedSetManifestShape(STTx const& tx) noexcept; +/** Return whether an unsigned SetManifest carries its one canonical Fee. + + Kept separate from the structural shape check so transactor preflight can + report temBAD_FEE precisely, while overlay ingress can reject fee variants + before either manifest signature is verified. +*/ +bool +hasCanonicalUnsignedSetManifestFee(STTx const& tx, Rules const& rules) noexcept; + /** Return the ruleset-fixed Fee for a manifest-authorized update. This is deliberately independent of the current ledger fee schedule: one diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index 7acea4d5f5..b3c4bf4a4f 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -86,6 +86,14 @@ checkValidity( Validity::SigBad, "Manifest-authorized envelope is not canonical"}; + // Fee is the last otherwise-malleable outer field. Pin it before + // verifying either manifest signature so changing eight cheap bytes + // cannot mint fresh txids that repeat expensive crypto. + if (!hasCanonicalUnsignedSetManifestFee(tx, rules)) + return { + Validity::SigBad, + "Manifest-authorized envelope has non-canonical fee"}; + // perform alternative signature check over manifest STObject const& manObj = const_cast(tx) .getField(sfManifest)