fix: Add zero keylet check in credential

include/xrpl/ledger/helpers/CredentialHelpers.h

@@ -7,6 +7,7 @@
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Rules.h>
 #include <xrpl/protocol/STArray.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STTx.h>
@@ -34,7 +35,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j);
 
 // Amendment and parameters checks for sfCredentialIDs field
 NotTEC
-checkFields(STTx const& tx, beast::Journal j);
+checkFields(STTx const& tx, Rules const& rules, beast::Journal j);
 
 // Accessing the ledger to check if provided credentials are valid. Do not use
 // in doApply (only in preclaim) since it does not remove expired credentials.

src/libxrpl/ledger/helpers/CredentialHelpers.cpp

@@ -22,6 +22,7 @@
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/digest.h>
 
+#include <algorithm>
 #include <cstdint>
 #include <expected>
 #include <limits>
@@ -52,6 +53,9 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j)
     for (auto const& h : arr)
     {
         // Credentials already checked in preclaim. Look only for expired here.
+        if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
+            return std::unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
+
         auto const k = keylet::credential(h);
         auto const sleCred = view.peek(k);
 
@@ -124,7 +128,7 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j)
 }
 
 NotTEC
-checkFields(STTx const& tx, beast::Journal j)
+checkFields(STTx const& tx, Rules const& rules, beast::Journal j)
 {
     if (!tx.isFieldPresent(sfCredentialIDs))
         return tesSUCCESS;
@@ -137,6 +141,13 @@ checkFields(STTx const& tx, beast::Journal j)
         return temMALFORMED;
     }
 
+    if (rules.enabled(fixCleanup3_4_0) &&
+        std::ranges::any_of(credentials, [](uint256 const& id) { return id.isZero(); }))
+    {
+        JLOG(j.trace()) << "Malformed transaction: zero credential ID.";
+        return temMALFORMED;
+    }
+
     std::unordered_set<uint256> duplicates;
     for (auto const& cred : credentials)
     {
@@ -160,6 +171,14 @@ valid(STTx const& tx, ReadView const& view, AccountID const& src, beast::Journal
     auto const& credIDs(tx.getFieldV256(sfCredentialIDs));
     for (auto const& h : credIDs)
     {
+        if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
+        {
+            // LCOV_EXCL_START
+            JLOG(j.trace()) << "Zero credential ID.";
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
+
         auto const sleCred = view.read(keylet::credential(h));
         if (!sleCred)
         {
@@ -234,6 +253,9 @@ authorizedDepositPreauth(ReadView const& view, STVector256 const& credIDs, Accou
     lifeExtender.reserve(credIDs.size());
     for (auto const& h : credIDs)
     {
+        if (view.rules().enabled(fixCleanup3_4_0) && h.isZero())
+            return tefINTERNAL;  // LCOV_EXCL_LINE
+
         auto sleCred = view.read(keylet::credential(h));
         if (!sleCred)            // already checked in preclaim
             return tefINTERNAL;  // LCOV_EXCL_LINE

src/libxrpl/tx/transactors/account/AccountDelete.cpp

@@ -50,7 +50,7 @@ AccountDelete::preflight(PreflightContext const& ctx)
         return temDST_IS_SRC;
     }
 
-    if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
+    if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
         return err;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/escrow/EscrowFinish.cpp

@@ -110,7 +110,7 @@ EscrowFinish::preflightSigValidated(PreflightContext const& ctx)
         }
     }
 
-    if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
+    if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
         return err;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/payment/Payment.cpp

@@ -281,7 +281,7 @@ Payment::preflight(PreflightContext const& ctx)
         }
     }
 
-    if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
+    if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
         return err;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp

@@ -87,7 +87,7 @@ PaymentChannelClaim::preflight(PreflightContext const& ctx)
             return temBAD_SIGNATURE;
     }
 
-    if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
+    if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
         return err;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp

@@ -82,7 +82,7 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx)
     if (hasAuditor && !isValidCiphertext(ctx.tx[sfAuditorEncryptedAmount]))
         return temBAD_CIPHERTEXT;
 
-    if (auto const err = credentials::checkFields(ctx.tx, ctx.j); !isTesSuccess(err))
+    if (auto const err = credentials::checkFields(ctx.tx, ctx.rules, ctx.j); !isTesSuccess(err))
         return err;
 
     return tesSUCCESS;

src/test/app/DepositAuth_test.cpp

@@ -934,6 +934,46 @@ struct DepositPreauth_test : public beast::unit_test::Suite
         }
     }
 
+    void
+    testZeroCredentialID(FeatureBitset features)
+    {
+        testcase("Zero credential ID");
+
+        using namespace jtx;
+
+        char const credType[] = "abcde";
+        Account const issuer{"issuer"};
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+
+        Env env(*this, features);
+
+        env.fund(XRP(5000), issuer, alice, bob);
+        env.close();
+
+        env(credentials::create(alice, issuer, credType));
+        env.close();
+        env(credentials::accept(alice, issuer, credType));
+        env.close();
+
+        auto const jv = credentials::ledgerEntry(env, alice, issuer, credType);
+        std::string const credIdx = jv[jss::result][jss::index].asString();
+
+        std::string const zeroIdx(64, '0');
+
+        // post-fixCleanup3_4_0: a zero ID is rejected by checkFields in
+        // preflight; pre-fixCleanup3_4_0, it will trigger assertion, so it is not testable.
+        env(pay(alice, bob, XRP(100)), credentials::Ids({zeroIdx}), Ter(temMALFORMED));
+        env.close();
+
+        env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx, zeroIdx}), Ter(temMALFORMED));
+        env.close();
+
+        // A valid credential succeeds
+        env(pay(alice, bob, XRP(100)), credentials::Ids({credIdx}));
+        env.close();
+    }
+
     void
     testCredentialsCreation()
     {
@@ -1446,6 +1486,7 @@ struct DepositPreauth_test : public beast::unit_test::Suite
         testPayment(supported - featureCredentials);
         testPayment(supported);
         testCredentialsPayment();
+        testZeroCredentialID(supported);
         testCredentialsCreation();
         testExpiredCreds();
         testSortingCredentials();