diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 23a36947ba..ea8ff2a8e8 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -34,6 +34,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FIX (HookAPISType, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(HookOnV2_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(OnChainManifests, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (HookMap, Supported::yes, VoteBehavior::DefaultYes) diff --git a/src/test/app/HookAPI_test.cpp b/src/test/app/HookAPI_test.cpp index 131520dbe5..af237d3cdb 100644 --- a/src/test/app/HookAPI_test.cpp +++ b/src/test/app/HookAPI_test.cpp @@ -3941,310 +3941,142 @@ public: auto const _source_object = *strUnHex("81140000000000000000000000000000000000000000"); + + std::vector> data_list = { + {// UINT16 Version + sfVersion.getCode(), + "10100001"}, + {// UINT32 Sequence + sfSequence.getCode(), + "2400000001"}, + // UINT64 + {// Uint64 ExchangeRate + sfExchangeRate.getCode(), + "360000000000000001"}, + {// UINT256 EmailHash + sfEmailHash.getCode(), + "4100000000000000000000000000000000"}, + {// UINT256 ObjectID + sfObjectID.getCode(), + "5E000000000000000000000000000000000000000000000000000000000000000" + "0"}, + {// Amount NativeAmount + sfAmount.getCode(), + "61999999999999999999999999999999999999999999999999999999999999999" + "999999999999999999999999999999999"}, + {// Amount IOUAmount + sfAmount.getCode(), + "614999999999999999"}, + {// Object AmountEntry + sfAmountEntry.getCode(), + "E05B614000000000000064E1"}, + {// Array Memos + sfMemos.getCode(), + "F9EA7D04DEADBEEFE1F1"}, + {// UINT8 TickSize + sfTickSize.getCode(), + "00101001"}, + {// UINT160 TakerPaysCurrency + sfTakerPaysCurrency.getCode(), + "01110000000000000000000000005553440000000000"}, + {// PathSet Paths + sfPaths.getCode(), + "0112300000000000000000000000005553440000000000054F6F784A58F9EFB0A" + "9EB90B83464F9D166461900"}, + {// Vector256 Amendments + sfAmendments.getCode(), + "03132042426C4D4F1009EE67080A9B7965B44656D7714D104A72F9B4369F97ABF" + "044EE"}, + {// Issue ClaimCurrency + sfClaimCurrency.getCode(), + "05180000000000000000000000005553440000000000054F6F784A58F9EFB0A9E" + "B90B83464F9D1664619"}, + {// XChainBridge Native-Native + sfXChainBridge.getCode(), + "011914AE123A8556F3CF91154711376AFB0F894F832B3D000000000000" + "000000000000000000000000000014AE123A8556F3CF91154711376AFB" + "0F894F832B3D0000000000000000000000000000000000000000"}, + {// Currency BaseAsset + sfBaseAsset.getCode(), + "011A0000000000000000000000005553440000000000"}, + }; + if (env.closed()->rules().enabled(fixHookAPISType)) + { + // encodings the legacy parser measures incorrectly + data_list.insert( + data_list.end(), + { + // MPT is not supported yet + // {// Issue ClaimCurrency MPT + // sfClaimCurrency.getCode(), + // "0518AE123A8556F3CF91154711376AFB0F894F832B3D0000000000" + // "00000000000000000000000000000100000001"}, + {// XChainBridge default door + IOU, Native + sfXChainBridge.getCode(), + "0119000000000000000000000000005553440000000000AE123A85" + "56F3CF91154711376AFB0F894F832B3D14AE123A8556F3CF911547" + "11376AFB0F894F832B3D00000000000000000000000000000000000" + "00000"}, + }); + } + bool const fixEnabled = env.closed()->rules().enabled(fixHookAPISType); + for (auto const& data : data_list) { - // use UINT16 auto source_object = _source_object; - auto field_object = *strUnHex("10100001"); - // Version: 1 + auto field_object = *strUnHex(data.second); + auto field_id = data.first; - auto const result = api.sto_emplace( - source_object, field_object, sfVersion.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), source_object.begin(), source_object.end()); - BEAST_EXPECT(result.value() == field_object); + // test invalid insert: trailing byte after the field + auto ft = field_object; + ft.push_back(0); + auto const result3 = api.sto_emplace(source_object, ft, field_id); + // legacy parser ignores trailing bytes in the field + if (fixEnabled) + BEAST_EXPECTS(!result3.has_value(), data.second); + else + BEAST_EXPECTS(result3.has_value(), data.second); - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfVersion.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); + // truncated field / source are only exercised after the fix; + // the legacy parser may read past the end of these buffers + if (!fixEnabled) + continue; + + auto const fo = Blob{field_object.begin(), field_object.end() - 1}; + auto const result1 = api.sto_emplace(source_object, fo, field_id); + BEAST_EXPECTS(!result1.has_value(), data.second); + + auto const so = + Blob{source_object.begin(), source_object.end() - 1}; + auto const result2 = api.sto_emplace(so, field_object, field_id); + BEAST_EXPECTS(!result2.has_value(), data.second); } + for (auto const& data : data_list) { - // use UINT32 auto source_object = _source_object; - auto field_object = *strUnHex("2400000001"); - // Sequence: 1 - - auto const result = api.sto_emplace( - source_object, field_object, sfSequence.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), source_object.begin(), source_object.end()); - BEAST_EXPECT(result.value() == field_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfSequence.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // use UINT64 - auto source_object = _source_object; - auto field_object = *strUnHex("360000000000000001"); - // ExchangeRate: 1 - - auto const result = api.sto_emplace( - source_object, field_object, sfExchangeRate.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), source_object.begin(), source_object.end()); - BEAST_EXPECT(result.value() == field_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfExchangeRate.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // use UINT128 - auto source_object = _source_object; - auto field_object = *strUnHex("4100000000000000000000000000000000"); - // EmailHash: 1 - - auto const result = api.sto_emplace( - source_object, field_object, sfEmailHash.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), source_object.begin(), source_object.end()); - BEAST_EXPECT(result.value() == field_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfEmailHash.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // use UINT256 - auto source_object = _source_object; - auto field_object = *strUnHex( - "5E000000000000000000000000000000000000000000000000000000000000" - "0000"); - // ObjectID: - // "0000000000000000000000000000000000000000000000000000000000000000" - - auto const result = api.sto_emplace( - source_object, field_object, sfObjectID.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), source_object.begin(), source_object.end()); - BEAST_EXPECT(result.value() == field_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfObjectID.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // use AMOUNT - auto source_object = _source_object; - - auto nativeamount = *strUnHex( - "61999999999999999999999999999999999999999999999999999999999999" - "999999999999999999999999999999999999"); - auto iouamount = *strUnHex("614999999999999999"); - - for (auto field_object : {nativeamount, iouamount}) - { - auto const result = api.sto_emplace( - source_object, field_object, sfAmount.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - field_object.insert( - field_object.end(), - source_object.begin(), - source_object.end()); - BEAST_EXPECT(result.value() == field_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfAmount.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT( - erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - } - - { - // OBJECT - auto source_object = _source_object; - auto field_object = *strUnHex("E05B614000000000000064E1"); - // {"AmountEntry": {"Amount": "100"}} - - auto const result = api.sto_emplace( - source_object, field_object, sfAmountEntry.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfAmountEntry.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // ARRAY - auto source_object = _source_object; - auto field_object = *strUnHex("F9EA7D04DEADBEEFE1F1"); - // {"Memos": [{"Memo":{ "MemoData": "DEADBEEF" }}]} + auto field_object = *strUnHex(data.second); + auto field_id = data.first; auto const result = - api.sto_emplace(source_object, field_object, sfMemos.getCode()); - BEAST_EXPECT(result.has_value()); + api.sto_emplace(source_object, field_object, field_id); + BEAST_EXPECTS(result.has_value(), data.second); + auto const r = result.value(); BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); + r.size() == source_object.size() + field_object.size()); + field_object.insert( + field_id < sfAccount.getCode() ? field_object.end() + : field_object.begin(), + source_object.begin(), + source_object.end()); + BEAST_EXPECT(r == field_object); - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfMemos.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // UINT8 - auto source_object = _source_object; - auto field_object = *strUnHex("00101001"); - // {"TickSize": 1} - - auto const result = api.sto_emplace( - source_object, field_object, sfTickSize.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfTickSize.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // UINT160 - auto source_object = _source_object; - auto field_object = - *strUnHex("01110000000000000000000000005553440000000000"); - // {"TakerPaysCurrency": "0000000000000000000000005553440000000000"} - - auto const result = api.sto_emplace( - source_object, field_object, sfTakerPaysCurrency.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfTakerPaysCurrency.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // PATHSET - auto source_object = _source_object; - auto field_object = *strUnHex( - "0112300000000000000000000000005553440000000000054F6F784A58F9EF" - "B0A9EB90B83464F9D166461900"); - // {"Paths": [[{ "currency": "USD", "issuer": - // "rVnYNK9yuxBz4uP8zC8LEFokM2nqH3poc" }]]} - - auto const result = - api.sto_emplace(source_object, field_object, sfPaths.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfPaths.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // VECTOR256 - auto source_object = _source_object; - auto field_object = *strUnHex( - "03132042426C4D4F1009EE67080A9B7965B44656D7714D104A72F9B4369F97" - "ABF044EE"); - // {"Amendments":["42426C4D4F1009EE67080A9B7965B44656D7714D104A72F9B4369F97ABF044EE"]} - - auto const result = api.sto_emplace( - source_object, field_object, sfAmendments.getCode()); - BEAST_EXPECT(result.has_value()); - BEAST_EXPECT( - result.value().size() == - source_object.size() + field_object.size()); - source_object.insert( - source_object.end(), field_object.begin(), field_object.end()); - BEAST_EXPECT(result.value() == source_object); - - auto const erase_result = api.sto_emplace( - result.value(), std::nullopt, sfAmendments.getCode()); - BEAST_EXPECT(erase_result.has_value()); - BEAST_EXPECT(erase_result.value().size() == _source_object.size()); - BEAST_EXPECT(erase_result.value() == _source_object); - } - - { - // UINT96 - } - - { - // UINT384 - } - - { - // UINT512 + // test erase + auto const erase_result = + api.sto_emplace(result.value(), std::nullopt, field_id); + BEAST_EXPECTS(erase_result.has_value(), data.second); + auto const er = erase_result.value(); + BEAST_EXPECT(er.size() == _source_object.size()); + BEAST_EXPECT(er == _source_object); } } @@ -4283,6 +4115,38 @@ public: PARSE_ERROR); } + { + // Invalid: wrapped array has an invalid end marker. + // should be "F9EA7D02BEEFE1F1" {Memos:[{Memo:{MemoData:"BEEF"}}]} + auto const invalid_end_marker = *strUnHex("F9EA7D02BEEFE100"); + auto const result = api.sto_subarray(invalid_end_marker, 0); + if (env.closed()->rules().enabled(fixHookAPISType)) + { + BEAST_EXPECT(!result.has_value()); + BEAST_EXPECT(result.error() == PARSE_ERROR); + } + else + { + BEAST_EXPECT(result.value() == std::make_pair(1u, 6u)); + } + } + + { + // Invalid: the first element is valid but a following element is + // truncated. + auto const invalid_tail = *strUnHex("F9EA7D02BEEFE1EA7D02BEEFF1"); + auto const result = api.sto_subarray(invalid_tail, 0); + if (env.closed()->rules().enabled(fixHookAPISType)) + { + BEAST_EXPECT(!result.has_value()); + BEAST_EXPECT(result.error() == PARSE_ERROR); + } + else + { + BEAST_EXPECT(result.value() == std::make_pair(1u, 6u)); + } + } + { // doesn't found // { Memos: [{Memo: {MemoData: "BEEF"}}] } @@ -4338,6 +4202,25 @@ public: PARSE_ERROR); } + { + // Invalid: the requested field is valid but a later field is + // truncated. + auto const data = *strUnHex( + "240000000181140000000000000000000000000000000000000000"); + auto const invalid_tail = Blob{data.begin(), data.end() - 1}; + auto const result = + api.sto_subfield(invalid_tail, sfSequence.getCode()); + if (env.closed()->rules().enabled(fixHookAPISType)) + { + BEAST_EXPECT(!result.has_value()); + BEAST_EXPECT(result.error() == PARSE_ERROR); + } + else + { + BEAST_EXPECT(result.value() == std::make_pair(1u, 4u)); + } + } + { // doesn't found // { Memo: {MemoData: "BEEF"} } @@ -4397,34 +4280,17 @@ public: // { AmountEntry: {Amount: "100"} } auto const amountEntry = *strUnHex("E05B614000000000000064E1"); - BEAST_EXPECT(api.sto_validate(memos).value() == true); - BEAST_EXPECT(api.sto_validate(amounts).value() == true); - BEAST_EXPECT(api.sto_validate(memo).value() == true); - BEAST_EXPECT(api.sto_validate(amountEntry).value() == true); + for (auto const& data : {memos, amounts, memo, amountEntry}) + { + // Valid data + BEAST_EXPECT(api.sto_validate(data).value() == true); - // Invalid data - BEAST_EXPECT( - api.sto_validate(Bytes{0xFF, 0xFF, 0xFF, 0xFF}).value() == false); - - Bytes const i_memos(&memos[0], &memos[memos.size() - 1]); - Bytes const i_amounts(&amounts[0], &amounts[amounts.size() - 1]); - Bytes const i_memo(&memo[0], &memo[memo.size() - 1]); - Bytes const i_amountEntry( - &amountEntry[0], &amountEntry[amountEntry.size() - 1]); - BEAST_EXPECT(api.sto_validate(i_memos).value() == false); - BEAST_EXPECT(api.sto_validate(i_amounts).value() == false); - BEAST_EXPECT(api.sto_validate(i_memo).value() == false); - BEAST_EXPECT(api.sto_validate(i_amountEntry).value() == false); - - Bytes const i2_memos(&memos[1], &memos[memos.size()]); - Bytes const i2_amounts(&amounts[1], &amounts[amounts.size()]); - Bytes const i2_memo(&memo[1], &memo[memo.size()]); - Bytes const i2_amountEntry( - &amountEntry[1], &amountEntry[amountEntry.size()]); - BEAST_EXPECT(api.sto_validate(i_memos).value() == false); - BEAST_EXPECT(api.sto_validate(i_amounts).value() == false); - BEAST_EXPECT(api.sto_validate(i_memo).value() == false); - BEAST_EXPECT(api.sto_validate(i_amountEntry).value() == false); + // Invalid data + auto const partial_data = Blob{data.begin(), data.end() - 1}; + BEAST_EXPECT(api.sto_validate(partial_data).value() == false); + auto const partial_data2 = Blob{data.begin() + 1, data.end()}; + BEAST_EXPECT(api.sto_validate(partial_data2).value() == false); + } } void @@ -4917,9 +4783,12 @@ public: test_state_set(features); test_sto_emplace(features); + test_sto_emplace(features - fixHookAPISType); // test_sto_erase(features); // tested in test_sto_emplace test_sto_subarray(features); + test_sto_subarray(features - fixHookAPISType); test_sto_subfield(features); + test_sto_subfield(features - fixHookAPISType); test_sto_validate(features); test_trace(features); diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index 4a5ff684e1..9911a0d214 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -12367,57 +12367,77 @@ public: )[test.hook]"]; HASH_WASM(hook); - for (auto isfixHookAPI20251128 : {true, false}) + for (bool const with20251128 : {true, false}) { - Env env{ - *this, - isfixHookAPI20251128 ? features | fixHookAPI20251128 - : features - fixHookAPI20251128}; - env.fund(XRP(10000), alice, bob); - env.close(); - - // install the hook on alice - env(ripple::test::jtx::hook( - alice, {{hso(hook_wasm, overrideFlag)}}, 0), - M("set sto_subarray"), - HSFEE); - env.close(); - EXPECT_HOOK_FEE(hook, 19); - - // 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 (isfixHookAPI20251128) + for (bool const withSType : {true, false}) { + auto feats = + features - fixHookAPI20251128 - fixHookAPISType; + if (with20251128) + feats = feats | fixHookAPI20251128; + if (withSType) + feats = feats | fixHookAPISType; + + Env env{*this, feats}; + env.fund(XRP(10000), alice, bob); + env.close(); + + // install the hook on alice + env(ripple::test::jtx::hook( + alice, {{hso(hook_wasm, overrideFlag)}}, 0), + M("set sto_subarray"), + HSFEE); + env.close(); + EXPECT_HOOK_FEE(hook, 19); + + // 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); 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)); + if (with20251128) + { + auto const position = 2; + auto const length = 12; + // unwrapped correctly: 1 element at pos 2, len 12 + BEAST_REQUIRE( + returnCode == + (doesntExistError + ((int64_t)position << 32) + + length)); + } + else if (withSType) + { + // legacy unwrap leaves 0x5C as a 32 byte field header, + // which is rejected as out of bounds. + // negative codes are stored as sign bit + magnitude + BEAST_REQUIRE( + returnCode == + (0x8000'0000'0000'0000ULL | + -(parseError + parseError))); + } + else + { + auto const position = 1; + auto const length = 33; + // legacy: 32 byte field overruns the 13 byte buffer + BEAST_REQUIRE( + returnCode == + (parseError + ((int64_t)position << 32) + length)); + } } } } diff --git a/src/xrpld/app/hook/detail/HookAPI.cpp b/src/xrpld/app/hook/detail/HookAPI.cpp index ca9d309764..e725504bd1 100644 --- a/src/xrpld/app/hook/detail/HookAPI.cpp +++ b/src/xrpld/app/hook/detail/HookAPI.cpp @@ -101,6 +101,14 @@ HookAPI::sto_subfield(Bytes const& data, uint32_t field_id) const if (data.size() < 2) return Unexpected(TOO_SMALL); + if (hookCtx.applyCtx.view().rules().enabled(fixHookAPISType)) + { + // validate the data + auto const valid = sto_validate(data); + if (!valid || !valid.value()) + return Unexpected(PARSE_ERROR); + } + unsigned char* start = const_cast(data.data()); unsigned char* upto = start; unsigned char* end = start + data.size(); @@ -165,6 +173,18 @@ HookAPI::sto_subarray(Bytes const& data, uint32_t index_id) const unsigned char* upto = start; unsigned char* end = start + data.size(); + if (hookCtx.applyCtx.view().rules().enabled(fixHookAPISType)) + { + // check if the array has valid trailing data + if ((*upto & 0xF0U) == 0xF0U && *(end - 1) != 0xF1U) + return Unexpected(PARSE_ERROR); + + // validate the array + auto const valid = sto_validate(data); + if (!valid || !valid.value()) + return Unexpected(PARSE_ERROR); + } + // unwrap the array if it is wrapped, // by removing a byte from the start and end // why here 0xF0? @@ -259,6 +279,13 @@ HookAPI::sto_emplace( return Unexpected(TOO_SMALL); } + if (hookCtx.applyCtx.view().rules().enabled(fixHookAPISType)) + { + auto const source_valid = sto_validate(source_object); + if (!source_valid || !source_valid.value()) + return Unexpected(PARSE_ERROR); + } + if (field_object.has_value() && hookCtx.applyCtx.view().rules().enabled(fixHookAPI20251128)) { @@ -279,6 +306,9 @@ HookAPI::sto_emplace( 0); if (!length) return Unexpected(PARSE_ERROR); + if (hookCtx.applyCtx.view().rules().enabled(fixHookAPISType) && + length.value() != field_object->size()) + return Unexpected(PARSE_ERROR); if ((type << 16) + field != field_id) { return Unexpected(PARSE_ERROR); @@ -3048,6 +3078,7 @@ HookAPI::get_stobject_length( type == STI_PATHSET && rules.enabled(featureHookAPISerializedType240)) { length = 0; + bool terminated = false; while (upto + length < end) { // iterate Path step @@ -3065,6 +3096,9 @@ HookAPI::get_stobject_length( if (flag & 0x20) // issuer length += 20; + if (rules.enabled(fixHookAPISType) && upto + length >= end) + return Unexpected(pe_unexpected_end); + int next_flag = *(upto + length); if (next_flag == 0x00 || next_flag == 0xff) // end of Path step @@ -3076,43 +3110,68 @@ HookAPI::get_stobject_length( if (lastflag == 0xff) continue; // continue byte else if (lastflag == 0x00) + { + terminated = true; break; // end byte + } else return Unexpected(pe_unexpected_end); } if (upto >= end) return Unexpected(pe_unexpected_end); + if (rules.enabled(fixHookAPISType) && !terminated) + return Unexpected(pe_unexpected_end); } else if (type == STI_ISSUE) { + bool const fix = rules.enabled(fixHookAPISType); auto zero20 = std::array{0}; // if first 20 byte is all zeros return 20 // else return 40 + if (fix && end - upto < 20) + return Unexpected(pe_unexpected_end); if (memcmp(upto, zero20.data(), 20) == 0) length = 20; + // MPT is not supported yet + // else if ( + // fix && end - upto >= 40 && + // memcmp(upto + 20, noAccount().data(), 20) == 0) + // length = 44; // MPT: Issuer + noAccount + Sequence else length = 40; } else if (type == STI_XCHAIN_BRIDGE) { + bool const fix = rules.enabled(fixHookAPISType); auto zero20 = std::array{0}; - // Lock Chain - length = 1; // Door Account1 prefix length - length += 20; // Door Account1 length - // Door Issue1 - if (memcmp(upto + length, zero20.data(), 20) == 0) - length += 20; // only Currency - else - length += 40; // Currency and Issue + length = 0; + for (int i = 0; i < 2; ++i) // Locking Chain, Issuing Chain + { + // Door Account + if (!fix) + length += 21; + else if (end - upto <= length) + return Unexpected(pe_unexpected_end); + else if (upto[length] == 0) + length += 1; // default (empty) account + else if (upto[length] == 20) + length += 21; + else + return Unexpected(pe_unexpected_end); - // Issuing Chain - length += 1; // Door Account2 prefix length - length += 20; // Door Account2 length - // Door Issue2 - if (memcmp(upto + length, zero20.data(), 20) == 0) - length += 20; // only Currency - else - length += 40; // Currency and Issue + // Door Issue + if (fix && end - upto < length + 20) + return Unexpected(pe_unexpected_end); + if (memcmp(upto + length, zero20.data(), 20) == 0) + length += 20; // only Currency + // MPT is not supported yet + // else if ( + // fix && end - upto >= length + 40 && + // memcmp(upto + length + 20, noAccount().data(), 20) == 0) + // length += 44; // MPT: Issuer + noAccount + Sequence + else + length += 40; // Currency and Issuer + } } if (length > -1) @@ -3129,6 +3188,10 @@ HookAPI::get_stobject_length( length, payload_start, payload_length); + + if (rules.enabled(fixHookAPISType) && length > end - upto) + return Unexpected(pe_unexpected_end); + return length + (upto - start); }