From 39353a65572a533c8ed8d9ba6661a46173dc3d79 Mon Sep 17 00:00:00 2001 From: tequ Date: Mon, 1 Dec 2025 11:48:30 +0900 Subject: [PATCH] Fix: Ensure `sto_subfield` correctly handles STO field values of 16 or more. (#647) --- src/ripple/app/hook/impl/applyHook.cpp | 19 ++++- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/SetHook_test.cpp | 113 ++++++++++++++++++++++--- src/test/app/SetHook_wasm.h | 59 +++++++++++++ 5 files changed, 180 insertions(+), 15 deletions(-) diff --git a/src/ripple/app/hook/impl/applyHook.cpp b/src/ripple/app/hook/impl/applyHook.cpp index 28cb93cc4..849f8d482 100644 --- a/src/ripple/app/hook/impl/applyHook.cpp +++ b/src/ripple/app/hook/impl/applyHook.cpp @@ -4233,10 +4233,25 @@ DEFINE_HOOK_FUNCTION( // unwrap the array if it is wrapped, // by removing a byte from the start and end + // why here 0xF0? + // STI_ARRAY = 0xF0 + // eg) Signers field value = 0x03 => 0xF3 + // eg) Amounts field value = 0x5C => 0xF0, 0x5C if ((*upto & 0xF0U) == 0xF0U) { - upto++; - end--; + if (view.rules().enabled(fixStoSubarray) && *upto == 0xF0U) + { + // field value > 15 + upto++; + upto++; + end--; + } + else + { + // field value <= 15 + upto++; + end--; + } } if (upto >= end) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 7d53d4ef2..e89193e94 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 = 91; +static constexpr std::size_t numFeatures = 92; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -379,6 +379,7 @@ extern uint256 const featureExtendedHookState; extern uint256 const fixCronStacking; extern uint256 const fixEtxnFeeBase; extern uint256 const fixStoEmplaceFieldIdCheck; +extern uint256 const fixStoSubarray; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 21e389e24..58b492750 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -485,6 +485,7 @@ REGISTER_FEATURE(ExtendedHookState, Supported::yes, VoteBehavior::De REGISTER_FIX (fixCronStacking, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fixEtxnFeeBase, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FIX (fixStoEmplaceFieldIdCheck, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixStoSubarray, 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/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index b67b26301..977f5a531 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -10574,14 +10574,15 @@ public: testcase("Test sto_subarray"); using namespace jtx; - Env env{*this, features}; - auto const bob = Account{"bob"}; auto const alice = Account{"alice"}; - env.fund(XRP(10000), alice); - env.fund(XRP(10000), bob); + { + Env env{*this, features}; - TestHook hook = wasm[R"[test.hook]( + env.fund(XRP(10000), alice); + env.fund(XRP(10000), bob); + + TestHook hook = wasm[R"[test.hook]( #include extern int32_t _g (uint32_t id, uint32_t maxiter); #define GUARD(maxiter) _g((1ULL << 31U) + __LINE__, (maxiter)+1) @@ -10632,14 +10633,102 @@ public: } )[test.hook]"]; - // install the hook on alice - env(ripple::test::jtx::hook(alice, {{hso(hook, overrideFlag)}}, 0), - M("set sto_subarray"), - HSFEE); - env.close(); + // install the hook on alice + env(ripple::test::jtx::hook(alice, {{hso(hook, overrideFlag)}}, 0), + M("set sto_subarray"), + HSFEE); + env.close(); - // invoke the hook - env(pay(bob, alice, XRP(1)), M("test sto_subarray"), fee(XRP(1))); + // invoke the hook + env(pay(bob, alice, XRP(1)), M("test sto_subarray"), fee(XRP(1))); + } + + { + TestHook hook = wasm[R"[test.hook]( + #include + extern int32_t _g (uint32_t id, uint32_t maxiter); + #define GUARD(maxiter) _g((1ULL << 31U) + __LINE__, (maxiter)+1) + extern int64_t accept (uint32_t read_ptr, uint32_t read_len, int64_t error_code); + extern int64_t sto_subarray( + uint32_t read_ptr, uint32_t read_len, uint32_t field_id); + #define DOESNT_EXIST -5 + + // { Amounts: [{AmountEntry: {Amount: "100"}}] } + uint8_t sto[] = + { + 0xF0U,0x5CU,0xE0U,0x5BU,0x61U,0x40U,0x00U,0x00U,0x00U,0x00U, + 0x00U,0x00U,0x64U,0xE1U,0xF1U + }; + + int64_t hook(uint32_t reserved ) + { + _g(1,1); + uint8_t hash[32]; + + // should be DOESNT_EXIST after Amendment enabled + int64_t result1 = sto_subarray(sto, sizeof(sto), 1); + + // should be position 2 length 12 + // before Amendment enabled, returns pos 1, len 33 + int64_t result2 = sto_subarray(sto, sizeof(sto), 0); + + accept(0,0,result1+result2); + } + )[test.hook]"]; + + for (auto isFixStoSubarray : {true, false}) + { + Env env{ + *this, + isFixStoSubarray ? features | fixStoSubarray + : features - fixStoSubarray}; + env.fund(XRP(10000), alice, bob); + env.close(); + + // install the hook on alice + env(ripple::test::jtx::hook( + alice, {{hso(hook, overrideFlag)}}, 0), + M("set sto_subarray"), + HSFEE); + env.close(); + + // invoke the hook + env(pay(bob, alice, XRP(1)), + M("test sto_subarray"), + fee(XRP(1))); + env.close(); + + auto const meta = env.meta(); + BEAST_REQUIRE(meta); + BEAST_REQUIRE(meta->isFieldPresent(sfHookExecutions)); + auto const hookExecutions = + meta->getFieldArray(sfHookExecutions); + BEAST_REQUIRE(hookExecutions.size() == 1); + auto const hookExecution = hookExecutions[0]; + BEAST_REQUIRE(hookExecution.isFieldPresent(sfHookReturnCode)); + auto const returnCode = + hookExecution.getFieldU64(sfHookReturnCode); + if (isFixStoSubarray) + { + auto const doesntExistError = -5; + auto const position = 2; + auto const length = 12; + BEAST_REQUIRE( + returnCode == + (doesntExistError + ((int64_t)position << 32) + + length)); + } + else + { + auto const parseError = -18; + auto const position = 1; + auto const length = 33; + BEAST_REQUIRE( + returnCode == + (parseError + ((int64_t)position << 32) + length)); + } + } + } } void diff --git a/src/test/app/SetHook_wasm.h b/src/test/app/SetHook_wasm.h index c21f407fb..73294c9fb 100644 --- a/src/test/app/SetHook_wasm.h +++ b/src/test/app/SetHook_wasm.h @@ -16746,6 +16746,65 @@ std::map> wasm = { 0x00U, }}, + /* ==== WASM: 79 ==== */ + {R"[test.hook]( + #include + extern int32_t _g (uint32_t id, uint32_t maxiter); + #define GUARD(maxiter) _g((1ULL << 31U) + __LINE__, (maxiter)+1) + extern int64_t accept (uint32_t read_ptr, uint32_t read_len, int64_t error_code); + extern int64_t sto_subarray( + uint32_t read_ptr, uint32_t read_len, uint32_t field_id); + #define DOESNT_EXIST -5 + + // { Amounts: [{AmountEntry: {Amount: "100"}}] } + uint8_t sto[] = + { + 0xF0U,0x5CU,0xE0U,0x5BU,0x61U,0x40U,0x00U,0x00U,0x00U,0x00U, + 0x00U,0x00U,0x64U,0xE1U,0xF1U + }; + + int64_t hook(uint32_t reserved ) + { + _g(1,1); + uint8_t hash[32]; + + // should be DOESNT_EXIST after Amendment enabled + int64_t result1 = sto_subarray(sto, sizeof(sto), 1); + + // should be position 2 length 12 + // before Amendment enabled, returns pos 1, len 33 + int64_t result2 = sto_subarray(sto, sizeof(sto), 0); + + accept(0,0,result1+result2); + } + )[test.hook]", + { + 0x00U, 0x61U, 0x73U, 0x6DU, 0x01U, 0x00U, 0x00U, 0x00U, 0x01U, 0x1AU, + 0x04U, 0x60U, 0x02U, 0x7FU, 0x7FU, 0x01U, 0x7FU, 0x60U, 0x03U, 0x7FU, + 0x7FU, 0x7FU, 0x01U, 0x7EU, 0x60U, 0x03U, 0x7FU, 0x7FU, 0x7EU, 0x01U, + 0x7EU, 0x60U, 0x01U, 0x7FU, 0x01U, 0x7EU, 0x02U, 0x2AU, 0x03U, 0x03U, + 0x65U, 0x6EU, 0x76U, 0x02U, 0x5FU, 0x67U, 0x00U, 0x00U, 0x03U, 0x65U, + 0x6EU, 0x76U, 0x0CU, 0x73U, 0x74U, 0x6FU, 0x5FU, 0x73U, 0x75U, 0x62U, + 0x61U, 0x72U, 0x72U, 0x61U, 0x79U, 0x00U, 0x01U, 0x03U, 0x65U, 0x6EU, + 0x76U, 0x06U, 0x61U, 0x63U, 0x63U, 0x65U, 0x70U, 0x74U, 0x00U, 0x02U, + 0x03U, 0x02U, 0x01U, 0x03U, 0x05U, 0x03U, 0x01U, 0x00U, 0x02U, 0x06U, + 0x27U, 0x06U, 0x7FU, 0x01U, 0x41U, 0x90U, 0x88U, 0x04U, 0x0BU, 0x7FU, + 0x00U, 0x41U, 0x8FU, 0x08U, 0x0BU, 0x7FU, 0x00U, 0x41U, 0x80U, 0x08U, + 0x0BU, 0x7FU, 0x00U, 0x41U, 0x90U, 0x88U, 0x04U, 0x0BU, 0x7FU, 0x00U, + 0x41U, 0x80U, 0x08U, 0x0BU, 0x7FU, 0x00U, 0x41U, 0x80U, 0x08U, 0x0BU, + 0x07U, 0x08U, 0x01U, 0x04U, 0x68U, 0x6FU, 0x6FU, 0x6BU, 0x00U, 0x03U, + 0x0AU, 0xC1U, 0x80U, 0x00U, 0x01U, 0xBDU, 0x80U, 0x00U, 0x01U, 0x01U, + 0x7EU, 0x41U, 0x01U, 0x41U, 0x01U, 0x10U, 0x80U, 0x80U, 0x80U, 0x80U, + 0x00U, 0x1AU, 0x41U, 0x00U, 0x41U, 0x00U, 0x41U, 0x80U, 0x88U, 0x80U, + 0x80U, 0x00U, 0x41U, 0x0FU, 0x41U, 0x01U, 0x10U, 0x81U, 0x80U, 0x80U, + 0x80U, 0x00U, 0x41U, 0x80U, 0x88U, 0x80U, 0x80U, 0x00U, 0x41U, 0x0FU, + 0x41U, 0x00U, 0x10U, 0x81U, 0x80U, 0x80U, 0x80U, 0x00U, 0x7CU, 0x10U, + 0x82U, 0x80U, 0x80U, 0x80U, 0x00U, 0x1AU, 0x20U, 0x01U, 0x0BU, 0x0BU, + 0x16U, 0x01U, 0x00U, 0x41U, 0x80U, 0x08U, 0x0BU, 0x0FU, 0xF0U, 0x5CU, + 0xE0U, 0x5BU, 0x61U, 0x40U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, + 0x64U, 0xE1U, 0xF1U, + }}, + /* ==== WASM: 79 ==== */ {R"[test.hook]( #include