From a2be55fbc92b24adcde9f3bc375807b39e151759 Mon Sep 17 00:00:00 2001
From: Ed Hennis <ed@ripple.com>
Date: Mon, 28 Jul 2025 19:02:52 -0400
Subject: [PATCH] Check LoanBrokerCoverWithdraw Destination and DestinationTag
 fields

- See also #5572 / e7a7bb8
---
 src/test/app/LoanBroker_test.cpp              | 22 ++++++++++++++++++-
 .../app/tx/detail/LoanBrokerCoverWithdraw.cpp | 14 ++++++++++--
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp
index 97e456903d..9f10db22d6 100644
--- a/src/test/app/LoanBroker_test.cpp
+++ b/src/test/app/LoanBroker_test.cpp
@@ -27,6 +27,7 @@
 #include <test/jtx/pay.h>
 #include <test/jtx/seq.h>
 #include <test/jtx/sig.h>
+#include <test/jtx/tag.h>
 #include <test/jtx/trust.h>
 #include <test/jtx/vault.h>
 
@@ -399,6 +400,17 @@ class LoanBroker_test : public beast::unit_test::suite
                     destination(bystander),
                     ter(expected));
             }
+
+            // Can not withdraw to the zero address
+            env(coverWithdraw(alice, keylet.key, vault.asset(1)),
+                destination(AccountID{}),
+                ter(temMALFORMED));
+
+            // If a destination tag is specified, a destination must be
+            // specified, too
+            env(coverWithdraw(alice, keylet.key, vault.asset(1)),
+                dtag(123),
+                ter(temMALFORMED));
             verifyCoverAmount(10);
 
             // Withdraw some of the cover amount
@@ -413,9 +425,17 @@ class LoanBroker_test : public beast::unit_test::suite
 
             // Withdraw some more. Send it to Evan. Very generous, considering
             // how much trouble he's been.
-            env(coverWithdraw(alice, keylet.key, vault.asset(2)),
+            env(coverWithdraw(alice, keylet.key, vault.asset(1)),
                 destination(evan));
             env.close();
+            verifyCoverAmount(7);
+
+            // Withdraw some more. Send it to Evan. Very generous, considering
+            // how much trouble he's been.
+            env(coverWithdraw(alice, keylet.key, vault.asset(1)),
+                destination(evan),
+                dtag(3));
+            env.close();
             verifyCoverAmount(6);
 
             if (!vault.asset.raw().native())
diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
index 5ec059d685..1f827cc7b8 100644
--- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
+++ b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
@@ -63,10 +63,20 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx)
         return temBAD_AMOUNT;
 
     if (auto const destination = ctx.tx[~sfDestination];
-        destination && *destination == beast::zero)
+        destination.has_value())
+    {
+        if (*destination == beast::zero)
+        {
+            JLOG(ctx.j.debug())
+                << "LoanBrokerCoverWithdraw: zero/empty destination account.";
+            return temMALFORMED;
+        }
+    }
+    else if (ctx.tx.isFieldPresent(sfDestinationTag))
     {
         JLOG(ctx.j.debug())
-            << "LoanBrokerCoverWithdraw: zero/empty destination account.";
+            << "LoanBrokerCoverWithdraw: sfDestinationTag is set but "
+               "sfDestination is not";
         return temMALFORMED;
     }