Improve VaultClawback checks

src/test/app/Vault_test.cpp

@@ -18,14 +18,17 @@
 //==============================================================================
 
 #include <test/jtx/Account.h>
+#include <test/jtx/Env.h>
 #include <test/jtx/fee.h>
 #include <test/jtx/mpt.h>
 #include <test/jtx/utility.h>
 #include <test/jtx/vault.h>
+#include <xrpl/basics/base_uint.h>
 #include <xrpl/protocol/Asset.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STNumber.h>
+#include <xrpl/protocol/TER.h>
 
 namespace ripple {
 
@@ -136,6 +139,20 @@ class Vault_test : public beast::unit_test::suite
                 env(tx);
             }
 
+            {
+                testcase(prefix + " fail to set zero domain");
+                auto tx = vault.set({.owner = owner, .id = keylet.key});
+                tx[sfDomainID] = to_string(base_uint<256>(beast::zero));
+                env(tx, ter(temMALFORMED));
+            }
+
+            {
+                testcase(prefix + " fail to set nonexistent domain");
+                auto tx = vault.set({.owner = owner, .id = keylet.key});
+                tx[sfDomainID] = to_string(base_uint<256>(42ul));
+                env(tx, ter(tecINVALID_DOMAIN));
+            }
+
             {
                 testcase(prefix + " fail to deposit more than maximum");
                 auto tx = vault.deposit(
@@ -189,7 +206,7 @@ class Vault_test : public beast::unit_test::suite
                      .id = keylet.key,
                      .holder = depositor,
                      .amount = asset(50)});
-                env(tx, ter(tecNO_PERMISSION));
+                env(tx, ter(temMALFORMED));
             }
 
             {
@@ -204,8 +221,8 @@ class Vault_test : public beast::unit_test::suite
 
             {
                 testcase(prefix + " clawback some");
-                auto code = asset.raw().native() ? ter(tecNO_PERMISSION)
-                                                 : ter(tesSUCCESS);
+                auto code =
+                    asset.raw().native() ? ter(temMALFORMED) : ter(tesSUCCESS);
                 auto tx = vault.clawback(
                     {.issuer = issuer,
                      .id = keylet.key,
@@ -219,10 +236,7 @@ class Vault_test : public beast::unit_test::suite
                 auto code = asset.raw().native() ? ter(tecNO_PERMISSION)
                                                  : ter(tesSUCCESS);
                 auto tx = vault.clawback(
-                    {.issuer = issuer,
-                     .id = keylet.key,
-                     .holder = depositor,
-                     .amount = std::nullopt});
+                    {.issuer = issuer, .id = keylet.key, .holder = depositor});
                 env(tx, code);
             }
 
@@ -272,6 +286,9 @@ class Vault_test : public beast::unit_test::suite
                 auto vault = env.vault();
                 env.fund(XRP(1000), issuer, owner, depositor);
                 env.close();
+                env(fset(issuer, asfAllowTrustLineClawback));
+                env.close();
+                env.require(flags(issuer, asfAllowTrustLineClawback));
 
                 PrettyAsset asset = setup(env, issuer, depositor);
                 testSequence(

src/test/jtx/vault.h

@@ -95,7 +95,7 @@ struct Vault
         Account issuer;
         uint256 id;
         Account holder;
-        std::optional<STAmount> amount;
+        std::optional<STAmount> amount{};
     };
 
     Json::Value

src/xrpld/app/tx/detail/VaultClawback.cpp

@@ -1,7 +1,7 @@
 //------------------------------------------------------------------------------
 /*
     This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2022 Ripple Labs Inc.
+    Copyright (c) 2025 Ripple Labs Inc.
 
     Permission to use, copy, modify, and/or distribute this software for any
     purpose  with  or without fee is hereby granted, provided that the above
@@ -41,10 +41,23 @@ VaultClawback::preflight(PreflightContext const& ctx)
     if (ctx.tx.getFlags() & tfUniversalMask)
         return temINVALID_FLAG;
 
-    // Note, zero amount is valid, it means "all". It is also the default.
+    AccountID const issuer = ctx.tx[sfAccount];
+    AccountID const holder = ctx.tx[sfHolder];
+
+    if (issuer == holder)
+        return temMALFORMED;
+
     auto const amount = ctx.tx[~sfAmount];
-    if (amount && *amount < beast::zero)
-        return temBAD_AMOUNT;
+    if (amount)
+    {
+        // Note, zero amount is valid, it means "all". It is also the default.
+        if (*amount < beast::zero)
+            return temBAD_AMOUNT;
+        else if (isXRP(amount->asset()))
+            return temMALFORMED;
+        else if (amount->asset().getIssuer() != issuer)
+            return temMALFORMED;
+    }
 
     return preflight2(ctx);
 }
@@ -57,6 +70,10 @@ VaultClawback::preclaim(PreclaimContext const& ctx)
         return tecOBJECT_NOT_FOUND;
 
     auto account = ctx.tx[sfAccount];
+    auto const issuer = ctx.view.read(keylet::account(account));
+    if (!issuer)
+        return tefINTERNAL;  // Transactor should have enforced this
+
     Asset const asset = vault->at(sfAsset);
     if (asset.native())
         return tecNO_PERMISSION;  // Cannot clawback XRP.
@@ -67,6 +84,14 @@ VaultClawback::preclaim(PreclaimContext const& ctx)
     if (amount && asset != amount->asset())
         return tecWRONG_ASSET;
 
+    std::uint32_t const issuerFlags = issuer->getFieldU32(sfFlags);
+
+    // If AllowTrustLineClawback is not set or NoFreeze is set, return no
+    // permission
+    if (!(issuerFlags & lsfAllowTrustLineClawback) ||
+        (issuerFlags & lsfNoFreeze))
+        return tecNO_PERMISSION;
+
     return tesSUCCESS;
 }