From 15c7ad6f787677c6af7fbb93ce7db9aeabc61731 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 14 Oct 2025 14:35:48 +0900 Subject: [PATCH] Fix Invalid Tx flags (#514) --- src/ripple/app/tx/impl/SetHook.cpp | 8 ++++++ src/ripple/app/tx/impl/SetSignerList.cpp | 8 ++++++ src/ripple/protocol/Feature.h | 3 ++- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/Import_test.cpp | 4 +-- src/test/app/MultiSign_test.cpp | 33 ++++++++++++++++++++++++ src/test/app/SetHook_test.cpp | 30 +++++++++++++++++++++ src/test/rpc/AccountTx_test.cpp | 10 +++---- 8 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/ripple/app/tx/impl/SetHook.cpp b/src/ripple/app/tx/impl/SetHook.cpp index d19a3337a..79e68e01d 100644 --- a/src/ripple/app/tx/impl/SetHook.cpp +++ b/src/ripple/app/tx/impl/SetHook.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -665,6 +666,13 @@ SetHook::preflight(PreflightContext const& ctx) if (!isTesSuccess(ret)) return ret; + if (ctx.rules.enabled(fixInvalidTxFlags) && + ctx.tx.getFlags() & tfUniversalMask) + { + JLOG(ctx.j.trace()) << "SetHook: Invalid flags set."; + return temINVALID_FLAG; + } + if (!ctx.tx.isFieldPresent(sfHooks)) { JLOG(ctx.j.trace()) diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 87f69c51d..71ed014c7 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -81,6 +82,13 @@ SetSignerList::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; + if (ctx.rules.enabled(fixInvalidTxFlags) && + (ctx.tx.getFlags() & tfUniversalMask)) + { + JLOG(ctx.j.trace()) << "SetSignerList: invalid flags."; + return temINVALID_FLAG; + } + auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j); if (!isTesSuccess(std::get<0>(result))) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 6982b866c..b52a78265 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 85; +static constexpr std::size_t numFeatures = 86; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -373,6 +373,7 @@ extern uint256 const fixProvisionalDoubleThreading; extern uint256 const featureClawback; extern uint256 const featureDeepFreeze; extern uint256 const featureIOUIssuerWeakTSH; +extern uint256 const fixInvalidTxFlags; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 9775b7d81..789423efa 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -479,6 +479,7 @@ REGISTER_FEATURE(Clawback, Supported::yes, VoteBehavior::De REGISTER_FIX (fixProvisionalDoubleThreading, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FEATURE(DeepFreeze, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(IOUIssuerWeakTSH, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixInvalidTxFlags, Supported::yes, VoteBehavior::DefaultYes); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/test/app/Import_test.cpp b/src/test/app/Import_test.cpp index 2bb56fbe9..057952836 100644 --- a/src/test/app/Import_test.cpp +++ b/src/test/app/Import_test.cpp @@ -5203,8 +5203,8 @@ class Import_test : public beast::unit_test::suite std::string ns_str = "CAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECAFECA" "FE"; - Json::Value jv = ripple::test::jtx::hook( - issuer, {{hso(createCodeHex)}}, hsfOVERRIDE | hsfCOLLECT); + Json::Value jv = + ripple::test::jtx::hook(issuer, {{hso(createCodeHex)}}, 0); jv[jss::Hooks][0U][jss::Hook][jss::HookNamespace] = ns_str; jv[jss::Hooks][0U][jss::Hook][jss::HookOn] = "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFDFFFFFFFFFFFFFFFFFFBFFF" diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 7668ef6be..1c4e537a3 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -1659,6 +1659,36 @@ public: BEAST_EXPECT(env.seq(alice) == aliceSeq + 1); } + void + test_signerListSetFlags(FeatureBitset features) + { + using namespace test::jtx; + + for (bool const withFixInvalidTxFlags : {false, true}) + { + Env env{ + *this, + withFixInvalidTxFlags ? features + : features - fixInvalidTxFlags}; + Account const alice{"alice"}; + + env.fund(XRP(1000), alice); + env.close(); + + bool const enabled = features[fixInvalidTxFlags]; + testcase( + std::string("SignerListSet flag, fix ") + + (withFixInvalidTxFlags ? "enabled" : "disabled")); + + ter const expected( + withFixInvalidTxFlags ? TER(temINVALID_FLAG) : TER(tesSUCCESS)); + env(signers(alice, 2, {{bogie, 1}, {ghost, 1}}), + expected, + txflags(tfPassive)); + env.close(); + } + } + void testAll(FeatureBitset features) { @@ -1695,6 +1725,9 @@ public: testAll(all - featureMultiSignReserve - featureExpandedSignerList); testAll(all - featureExpandedSignerList); testAll(all); + + test_signerListSetFlags(all); + test_amendmentTransition(); } }; diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index 3fb16176b..75caade5e 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -364,6 +364,35 @@ public: } } + void + testInvalidTxFlags(FeatureBitset features) + { + testcase("Checks invalid tx flags"); + using namespace jtx; + + for (bool const withFixInvalidTxFlags : {false, true}) + { + Env env{ + *this, + withFixInvalidTxFlags ? features + : features - fixInvalidTxFlags}; + + auto const alice = Account{"alice"}; + env.fund(XRP(10000), alice); + env.close(); + + Json::Value jv = + ripple::test::jtx::hook(alice, {{{hso_delete()}}}, 0); + jv[jss::Flags] = tfUniversalMask; + + env(jv, + M("Invalid SetHook flags"), + HSFEE, + withFixInvalidTxFlags ? ter(temINVALID_FLAG) : ter(tesSUCCESS)); + env.close(); + } + } + void testGrants(FeatureBitset features) { @@ -12737,6 +12766,7 @@ public: testHooksOwnerDir(features); testHooksDisabled(features); testTxStructure(features); + testInvalidTxFlags(features); testInferHookSetOperation(); testParams(features); testGrants(features); diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 85dd2978d..aaed4defe 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -523,8 +523,7 @@ class AccountTx_test : public beast::unit_test::suite "0B"; Json::Value jhv = hso(updateHookHex); jhv[jss::Flags] = hsfOVERRIDE; - Json::Value jv = - ripple::test::jtx::hook(account, {{jhv}}, hsfOVERRIDE); + Json::Value jv = ripple::test::jtx::hook(account, {{jhv}}, 0); return jv; }; env(updateHook(alice), HSFEE, sig(alie)); @@ -553,8 +552,7 @@ class AccountTx_test : public beast::unit_test::suite "000000"; jhv[jss::HookNamespace] = to_string(uint256{beast::zero}); jhv[jss::HookHash] = to_string(hookHash); - Json::Value jv = - ripple::test::jtx::hook(account, {{jhv}}, hsfOVERRIDE); + Json::Value jv = ripple::test::jtx::hook(account, {{jhv}}, 0); return jv; }; uint256 const hid = hh(env, alice); @@ -563,8 +561,8 @@ class AccountTx_test : public beast::unit_test::suite // Delete Hook auto deleteHook = [](test::jtx::Account const& account) { - Json::Value jv = ripple::test::jtx::hook( - account, {{hso_delete()}}, hsfOVERRIDE); + Json::Value jv = + ripple::test::jtx::hook(account, {{hso_delete()}}, 0); return jv; }; env(deleteHook(alice), HSFEE, sig(alie));