fix(manifest): reject fee variants before crypto

This commit is contained in:
Nicholas Dudfield
2026-08-31 13:29:14 +07:00
parent 619d6a16ff
commit ce32b75db1
5 changed files with 52 additions and 5 deletions

View File

@@ -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(

View File

@@ -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

View File

@@ -122,6 +122,22 @@ hasCanonicalUnsignedSetManifestShape(STTx const& tx) noexcept
}
}
bool
hasCanonicalUnsignedSetManifestFee(STTx const& tx, Rules const& rules) noexcept
{
try
{
auto const& manifest =
const_cast<STTx&>(tx).getField(sfManifest).downcast<STObject>();
return tx[sfFee].xrp() ==
canonicalUnsignedSetManifestFee(rules, manifest);
}
catch (std::exception const&)
{
return false;
}
}
std::optional<std::uint32_t>
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

View File

@@ -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

View File

@@ -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<ripple::STTx&>(tx)
.getField(sfManifest)