From 413b82397616238e86b9c4ae3fd79378d725f51a Mon Sep 17 00:00:00 2001
From: Alex Kremer <akremer@ripple.com>
Date: Wed, 9 Jul 2025 13:43:39 +0100
Subject: [PATCH] fix: ASAN stack-buffer-overflow in
 NFTHelpersTest_NFTDataFromLedgerObject (#2306)

---
 src/etl/NFTHelpers.cpp                 | 11 ++++++++---
 tests/common/util/BinaryTestObject.cpp |  9 +++++++--
 tests/unit/etl/NFTHelpersTests.cpp     | 17 ++++++++++-------
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/etl/NFTHelpers.cpp b/src/etl/NFTHelpers.cpp
index 4a668056..0f36105d 100644
--- a/src/etl/NFTHelpers.cpp
+++ b/src/etl/NFTHelpers.cpp
@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include "data/DBHelpers.hpp"
+#include "util/Assert.hpp"
 
 #include <fmt/format.h>
 #include <xrpl/basics/base_uint.h>
@@ -359,14 +360,18 @@ getNFTDataFromTx(ripple::TxMeta const& txMeta, ripple::STTx const& sttx)
 std::vector<NFTsData>
 getNFTDataFromObj(std::uint32_t const seq, std::string const& key, std::string const& blob)
 {
-    std::vector<NFTsData> nfts;
-    ripple::STLedgerEntry const sle =
+    // https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0020-non-fungible-tokens#tokenpage-id-format
+    ASSERT(key.size() == ripple::uint256::size(), "The size of the key (token) is expected to fit uint256 exactly");
+
+    auto const sle =
         ripple::STLedgerEntry(ripple::SerialIter{blob.data(), blob.size()}, ripple::uint256::fromVoid(key.data()));
 
     if (sle.getFieldU16(ripple::sfLedgerEntryType) != ripple::ltNFTOKEN_PAGE)
-        return nfts;
+        return {};
 
     auto const owner = ripple::AccountID::fromVoid(key.data());
+    std::vector<NFTsData> nfts;
+
     for (ripple::STObject const& node : sle.getFieldArray(ripple::sfNFTokens))
         nfts.emplace_back(node.getFieldH256(ripple::sfNFTokenID), seq, owner, node.getFieldVL(ripple::sfURI));
 
diff --git a/tests/common/util/BinaryTestObject.cpp b/tests/common/util/BinaryTestObject.cpp
index f4d0df25..d03a2ef6 100644
--- a/tests/common/util/BinaryTestObject.cpp
+++ b/tests/common/util/BinaryTestObject.cpp
@@ -148,11 +148,16 @@ createObjectWithTwoNFTs()
 
     auto const nftPage = createNftTokenPage({{kNFT_ID, url1}, {kNFT_ID2, url2}}, std::nullopt);
     auto const serializerNftPage = nftPage.getSerializer();
-
     auto const account = getAccountIdWithString(kACCOUNT);
+
+    // key is a token made up from owner's account ID followed by unused (in Clio) value described here:
+    // https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0020-non-fungible-tokens#tokenpage-id-format
+    auto constexpr kEXTRA_BYTES = "000000000000";
+    auto const key = std::string(std::begin(account), std::end(account)) + kEXTRA_BYTES;
+
     return {
         .key = {},
-        .keyRaw = std::string(reinterpret_cast<char const*>(account.data()), ripple::AccountID::size()),
+        .keyRaw = key,
         .data = {},
         .dataRaw =
             std::string(static_cast<char const*>(serializerNftPage.getDataPtr()), serializerNftPage.getDataLength()),
diff --git a/tests/unit/etl/NFTHelpersTests.cpp b/tests/unit/etl/NFTHelpersTests.cpp
index 619dcf6f..7164511e 100644
--- a/tests/unit/etl/NFTHelpersTests.cpp
+++ b/tests/unit/etl/NFTHelpersTests.cpp
@@ -26,6 +26,7 @@
 #include <xrpl/basics/Blob.h>
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/strHex.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STObject.h>
@@ -471,17 +472,19 @@ TEST_F(NFTHelpersTest, NFTDataFromLedgerObject)
     ripple::Blob const uri1Blob(url1.begin(), url1.end());
     ripple::Blob const uri2Blob(url2.begin(), url2.end());
 
+    auto const account = getAccountIdWithString(kACCOUNT);
     auto const nftPage = createNftTokenPage({{kNFT_ID, url1}, {kNFT_ID2, url2}}, std::nullopt);
     auto const serializerNftPage = nftPage.getSerializer();
+    auto const blob =
+        std::string(static_cast<char const*>(serializerNftPage.getDataPtr()), serializerNftPage.getDataLength());
 
-    int constexpr kSEQ{5};
-    auto const account = getAccountIdWithString(kACCOUNT);
+    // key is a token made up from owner's account ID followed by unused (in Clio) value described here:
+    // https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0020-non-fungible-tokens#tokenpage-id-format
+    auto constexpr kEXTRA_BYTES = "000000000000";
+    auto const key = std::string(std::begin(account), std::end(account)) + kEXTRA_BYTES;
 
-    auto const nftDatas = etl::getNFTDataFromObj(
-        kSEQ,
-        std::string(reinterpret_cast<char const*>(account.data()), ripple::AccountID::size()),
-        std::string(static_cast<char const*>(serializerNftPage.getDataPtr()), serializerNftPage.getDataLength())
-    );
+    uint32_t constexpr kSEQ{5};
+    auto const nftDatas = etl::getNFTDataFromObj(kSEQ, key, blob);
 
     EXPECT_EQ(nftDatas.size(), 2);
     EXPECT_EQ(nftDatas[0].tokenID, ripple::uint256(kNFT_ID));