diff --git a/src/libxrpl/protocol/Indexes.cpp b/src/libxrpl/protocol/Indexes.cpp index ae29bd3297..043c2626e7 100644 --- a/src/libxrpl/protocol/Indexes.cpp +++ b/src/libxrpl/protocol/Indexes.cpp @@ -116,19 +116,36 @@ getBookBase(Book const& book) return getIndexHash( LedgerNameSpace::BookDir, in.currency, out.currency, in.account, out.account); } + // The three MPT-involving branches are new under MPTokensV2 and + // each gets a 1-byte discriminator to prevent preimage collisions + // between branches: the (Issue,MPT) and (MPT,Issue) preimages + // are both 64 bytes of raw concatenation, so without a + // per-branch tag chosen Currency / MPTID / AccountID values can + // align byte-for-byte and produce the same BookDir keylet for + // two distinct markets. (Issue,Issue) is left untagged to + // preserve existing mainnet order-book keylets. else if constexpr (std::is_same_v && std::is_same_v) { return getIndexHash( - LedgerNameSpace::BookDir, in.currency, out.getMptID(), in.account); + LedgerNameSpace::BookDir, + std::uint8_t{0x01}, + in.currency, + out.getMptID(), + in.account); } else if constexpr (std::is_same_v && std::is_same_v) { return getIndexHash( - LedgerNameSpace::BookDir, in.getMptID(), out.currency, out.account); + LedgerNameSpace::BookDir, + std::uint8_t{0x02}, + in.getMptID(), + out.currency, + out.account); } else { - return getIndexHash(LedgerNameSpace::BookDir, in.getMptID(), out.getMptID()); + return getIndexHash( + LedgerNameSpace::BookDir, std::uint8_t{0x03}, in.getMptID(), out.getMptID()); } }, book.in.value(), diff --git a/src/test/app/OfferMPT_test.cpp b/src/test/app/OfferMPT_test.cpp index 0496d69fef..811aa47ba0 100644 --- a/src/test/app/OfferMPT_test.cpp +++ b/src/test/app/OfferMPT_test.cpp @@ -5102,6 +5102,73 @@ public: } } + // getBookBase hashes raw concatenations of fixed-width fields, so the + // (Issue,MPT) preimage `currency(20)||mptID(24)||account(20)` and the + // (MPT,Issue) preimage `mptID(24)||currency(20)||account(20)` are both + // 64 bytes and collide when the bytes align. An attacker picks the IOU + // currency, reuses an IOU issuer, and grinds an MPT issuer / sequence; + // the per-branch discriminator in getBookBase blocks this. + void + testBookBaseMixedAssetCollision(FeatureBitset /*features*/) + { + testcase("getBookBase: (Issue,MPT) vs (MPT,Issue) preimage collision"); + + // Construction recipe: + // issuerB last 4 bytes == seq_A; mptID_B = BE(5) || issuerB + // currencyA == mptID_B[0..19] = BE(5) || issuerB[0..15] + // issuerA == currencyB (both 20-byte all-0xBB) + // sharedIOUIssuer == acct_A == acct_B + AccountID issuerB; + AccountID issuerA; + Currency currencyB; + Currency currencyA; + AccountID sharedIOUIssuer; + BEAST_EXPECT(issuerB.parseHex("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA00000007")); + BEAST_EXPECT(issuerA.parseHex("BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB")); + BEAST_EXPECT(currencyB.parseHex("BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB")); + BEAST_EXPECT(currencyA.parseHex("00000005AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")); + BEAST_EXPECT(sharedIOUIssuer.parseHex("CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC")); + + Book const bookA{ + Asset{Issue{currencyA, sharedIOUIssuer}}, + Asset{MPTIssue{0x00000007u, issuerA}}, + std::nullopt}; + Book const bookB{ + Asset{MPTIssue{0x00000005u, issuerB}}, + Asset{Issue{currencyB, sharedIOUIssuer}}, + std::nullopt}; + + BEAST_EXPECT(bookA != bookB); + BEAST_EXPECT(getBookBase(bookA) != getBookBase(bookB)); + } + + // (MPT,MPT) bodies are 48 bytes and can't length-match the 64-byte + // mixed branches, but tag them too for symmetry/future-proofing; this + // test also pins the directional asymmetry of an (MPT,MPT) book. + void + testBookBaseMptMptDistinct(FeatureBitset /*features*/) + { + testcase("getBookBase: (MPT,MPT) distinguishes from mixed branches"); + + AccountID issuerX; + AccountID issuerY; + Currency currency; + AccountID iouIssuer; + BEAST_EXPECT(issuerX.parseHex("1111111111111111111111111111111111111111")); + BEAST_EXPECT(issuerY.parseHex("2222222222222222222222222222222222222222")); + BEAST_EXPECT(currency.parseHex("3333333333333333333333333333333333333333")); + BEAST_EXPECT(iouIssuer.parseHex("4444444444444444444444444444444444444444")); + + Asset const mptX{MPTIssue{1u, issuerX}}; + Asset const mptY{MPTIssue{2u, issuerY}}; + Book const mptBook{mptX, mptY, std::nullopt}; + Book const mixedBook{mptX, Asset{Issue{currency, iouIssuer}}, std::nullopt}; + Book const reversedMptBook{mptY, mptX, std::nullopt}; + + BEAST_EXPECT(getBookBase(mptBook) != getBookBase(mixedBook)); + BEAST_EXPECT(getBookBase(mptBook) != getBookBase(reversedMptBook)); + } + void testAll(FeatureBitset features) { @@ -5160,6 +5227,8 @@ public: testTickSize(features); testBookOffersMPTFunding(features); testAutoCreateReserve(features); + testBookBaseMixedAssetCollision(features); + testBookBaseMptMptDistinct(features); } void