fix: Check trustline limits for share-denominated vault withdrawals

When a VaultWithdraw specifies the amount in shares (MPT) rather than
assets, convert to the equivalent asset amount before calling
canWithdraw. Previously the limit check was skipped for share-denominated
withdrawals, allowing funds to exceed the destination's trustline limit.

src/libxrpl/tx/transactors/oracle/OracleSet.cpp

@@ -98,7 +98,7 @@ OracleSet::preclaim(PreclaimContext const& ctx)
         return !v || *v == (*sle)[field];
     };
 
-    std::uint32_t adjustReserve = 0;
+    std::int8_t adjustReserve = 0;
     if (sle)
     {
         // update

src/libxrpl/tx/transactors/vault/VaultClawback.cpp

@@ -11,6 +11,7 @@
 #include <xrpl/tx/transactors/vault/VaultClawback.h>
 
 #include <optional>
+#include <utility>
 
 namespace xrpl {
 NotTEC
@@ -223,40 +224,41 @@ VaultClawback::assetsToClawback(
     auto const assetsAvailable = vault->at(sfAssetsAvailable);
     auto const mptIssuanceID = *vault->at(sfShareMPTID);
     MPTIssue const share{mptIssuanceID};
-
-    if (clawbackAmount == beast::zero)
-    {
-        auto const sharesDestroyed = accountHolds(
-            view(),
-            holder,
-            share,
-            FreezeHandling::fhIGNORE_FREEZE,
-            AuthHandling::ahIGNORE_AUTH,
-            j_);
-        auto const maybeAssets = sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-
-        return std::make_pair(*maybeAssets, sharesDestroyed);
-    }
-
     STAmount sharesDestroyed;
-    STAmount assetsRecovered = clawbackAmount;
+    STAmount assetsRecovered;
+
     try
     {
+        if (clawbackAmount == beast::zero)
+        {
+            sharesDestroyed = accountHolds(
+                view(),
+                holder,
+                share,
+                FreezeHandling::fhIGNORE_FREEZE,
+                AuthHandling::ahIGNORE_AUTH,
+                j_);
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
+
+            assetsRecovered = *maybeAssets;
+        }
+        else
         {
             auto const maybeShares =
-                assetsToSharesWithdraw(vault, sleShareIssuance, assetsRecovered);
+                assetsToSharesWithdraw(vault, sleShareIssuance, clawbackAmount);
             if (!maybeShares)
                 return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
             sharesDestroyed = *maybeShares;
+
+            auto const maybeAssets =
+                sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
+            if (!maybeAssets)
+                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
+            assetsRecovered = *maybeAssets;
         }
-
-        auto const maybeAssets = sharesToAssetsWithdraw(vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-        assetsRecovered = *maybeAssets;
-
         // Clamp to maximum.
         if (assetsRecovered > *assetsAvailable)
         {

src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp

@@ -41,10 +41,20 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
     if (!vault)
         return tecNO_ENTRY;
 
-    auto const assets = ctx.tx[sfAmount];
+    auto const mptIssuanceID = vault->at(sfShareMPTID);
+    auto const sleIssuance = ctx.view.read(keylet::mptIssuance(mptIssuanceID));
+    if (!sleIssuance)
+    {
+        // LCOV_EXCL_START
+        JLOG(ctx.j.error()) << "VaultWithdraw: missing issuance of vault shares.";
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
+
+    auto const amount = ctx.tx[sfAmount];
     auto const vaultAsset = vault->at(sfAsset);
     auto const vaultShare = vault->at(sfShareMPTID);
-    if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
+    if (amount.asset() != vaultAsset && amount.asset() != vaultShare)
         return tecWRONG_ASSET;
 
     auto const& vaultAccount = vault->at(sfAccount);
@@ -65,8 +75,26 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
         // LCOV_EXCL_STOP
     }
 
-    if (auto const ret = canWithdraw(ctx.view, ctx.tx))
-        return ret;
+    if (amount.asset() == vaultShare)
+    {
+        // If the user specified shares, we need to first convert them to asset amount before
+        // checking whether they can be withdrawn
+        auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, amount);
+        if (!maybeAssets)
+            return tecINTERNAL;  // LCOV_EXCL_LINE
+
+        auto const from = ctx.tx[sfAccount];
+        auto const to = ctx.tx[~sfDestination].value_or(from);
+
+        if (auto const ret = canWithdraw(
+                ctx.view, from, to, *maybeAssets, ctx.tx.isFieldPresent(sfDestinationTag)))
+            return ret;
+    }
+    else
+    {
+        if (auto const ret = canWithdraw(ctx.view, ctx.tx))
+            return ret;
+    }
 
     // If sending to Account (i.e. not a transfer), we will also create (only
     // if authorized) a trust line or MPToken as needed, in doApply().

src/test/app/Vault_test.cpp

@@ -4797,6 +4797,7 @@ class Vault_test : public beast::unit_test::suite
 
             auto const& vaultSle = env.le(vaultKeylet);
             BEAST_EXPECT(vaultSle != nullptr);
+            env.memoize(Account("vault", vaultSle->at(sfAccount)));
             env(vault.deposit(
                     {.depositor = depositor, .id = vaultKeylet.key, .amount = asset(100)}),
                 ter(tesSUCCESS));
@@ -4953,6 +4954,126 @@ class Vault_test : public beast::unit_test::suite
                     }),
                     ter(tesSUCCESS));
             }
+
+            {
+                testcase(
+                    "VaultClawback (asset) - " + prefix +
+                    " zero-amount clawback clamped with outstanding loan");
+                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer);
+
+                auto const vaultSle = env.le(vaultKeylet);
+                BEAST_EXPECT(vaultSle != nullptr);
+                if (!vaultSle)
+                    return;
+
+                PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID));
+
+                // Create a loan broker backed by this vault
+                auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner));
+                env(set(owner, vaultKeylet.key));
+                env.close();
+
+                // Depositor borrows 40 units, reducing assetsAvailable to 60
+                // while assetsTotal stays at 100
+                env(set(depositor, brokerKeylet.key, asset(40).value()),
+                    loan::interestRate(TenthBips32(0)),
+                    gracePeriod(60),
+                    paymentInterval(120),
+                    paymentTotal(10),
+                    sig(sfCounterpartySignature, owner),
+                    fee(env.current()->fees().base * 2),
+                    ter(tesSUCCESS));
+                env.close();
+
+                {
+                    auto const sle = env.le(vaultKeylet);
+                    BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value());
+                    BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value());
+                }
+
+                auto const sharesBefore = env.balance(depositor, shares.raw().get<MPTIssue>());
+
+                // Zero-amount clawback (= "clawback all") should succeed,
+                // clamped to assetsAvailable (60) rather than the full
+                // share value (100).
+                env(vault.clawback({
+                        .issuer = issuer,
+                        .id = vaultKeylet.key,
+                        .holder = depositor,
+                    }),
+                    ter(tesSUCCESS));
+                env.close();
+
+                // Only 60 assets clawed back; loan's 40 still outstanding
+                {
+                    auto const sle = env.le(vaultKeylet);
+                    BEAST_EXPECT(sle != nullptr);
+                    BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value());
+                    BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value());
+
+                    // 60 of 100 shares destroyed (1:1 ratio), 40 remain
+                    auto const sharesAfter = env.balance(depositor, shares.raw().get<MPTIssue>());
+                    BEAST_EXPECT(sharesAfter == shares(Number{4, sle->at(sfScale) + 1}));
+                }
+            }
+
+            {
+                testcase(
+                    "VaultClawback (asset) - " + prefix +
+                    " non-zero clawback clamped with outstanding loan");
+                auto [vault, vaultKeylet] = setupVault(asset, owner, depositor, issuer);
+
+                auto const vaultSle = env.le(vaultKeylet);
+                BEAST_EXPECT(vaultSle != nullptr);
+                if (!vaultSle)
+                    return;
+                PrettyAsset shares = MPTIssue(vaultSle->at(sfShareMPTID));
+
+                // Create a loan broker backed by this vault
+                auto const brokerKeylet = keylet::loanbroker(owner.id(), env.seq(owner));
+                env(set(owner, vaultKeylet.key));
+                env.close();
+
+                // Depositor borrows 40 units
+                env(set(depositor, brokerKeylet.key, asset(40).value()),
+                    loan::interestRate(TenthBips32(0)),
+                    gracePeriod(60),
+                    paymentInterval(120),
+                    paymentTotal(10),
+                    sig(sfCounterpartySignature, owner),
+                    fee(env.current()->fees().base * 2),
+                    ter(tesSUCCESS));
+                env.close();
+
+                {
+                    auto const sle = env.le(vaultKeylet);
+                    BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(60).value());
+                    BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(100).value());
+                }
+
+                auto const sharesBefore = env.balance(depositor, shares);
+
+                // Request 100 but only 60 available — clamped to 60
+                env(vault.clawback({
+                        .issuer = issuer,
+                        .id = vaultKeylet.key,
+                        .holder = depositor,
+                        .amount = asset(100).value(),
+                    }),
+                    ter(tesSUCCESS));
+                env.close();
+
+                {
+                    auto const sle = env.le(vaultKeylet);
+                    BEAST_EXPECT(sle != nullptr);
+                    BEAST_EXPECT(sle->at(sfAssetsAvailable) == asset(0).value());
+                    BEAST_EXPECT(sle->at(sfAssetsTotal) == asset(40).value());
+                }
+
+                auto const sharesAfter = env.balance(depositor, shares);
+                BEAST_EXPECT(sharesAfter < sharesBefore);
+                BEAST_EXPECT(sharesAfter > shares(0));
+            }
         };
 
         Account owner{"alice"};
@@ -5229,6 +5350,79 @@ class Vault_test : public beast::unit_test::suite
         }
     }
 
+    // Reproduction: canWithdraw IOU limit check bypassed when
+    // withdrawal amount is specified in shares (MPT) rather than in assets.
+    void
+    testBug6_LimitBypassWithShares()
+    {
+        using namespace test::jtx;
+        testcase("Bug6 - limit bypass with share-denominated withdrawal");
+
+        Env env{*this, testable_amendments() | featureSingleAssetVault};
+        Account const owner{"owner"};
+        Account const issuer{"issuer"};
+        Account const depositor{"depositor"};
+        Account const charlie{"charlie"};
+        Vault vault{env};
+
+        env.fund(XRP(1000), issuer, owner, depositor, charlie);
+        env(fset(issuer, asfAllowTrustLineClawback));
+        env.close();
+
+        PrettyAsset const asset = issuer["IOU"];
+        env.trust(asset(1000), owner);
+        env.trust(asset(1000), depositor);
+        env(pay(issuer, owner, asset(200)));
+        env(pay(issuer, depositor, asset(200)));
+        env.close();
+
+        // Charlie gets a LOW trustline limit of 5
+        env.trust(asset(5), charlie);
+        env.close();
+
+        auto const [tx, keylet] = vault.create({.owner = owner, .asset = asset});
+        env(tx);
+        env.close();
+
+        auto const depositTx =
+            vault.deposit({.depositor = depositor, .id = keylet.key, .amount = asset(100)});
+        env(depositTx);
+        env.close();
+
+        // Get the share MPT info
+        auto const vaultSle = env.le(keylet);
+        if (!BEAST_EXPECT(vaultSle))
+            return;
+        auto const mptIssuanceID = vaultSle->at(sfShareMPTID);
+        MPTIssue const shares(mptIssuanceID);
+        PrettyAsset const share(shares);
+
+        // CONTROL: Withdraw 10 IOU (asset-denominated) to charlie.
+        // Charlie's limit is 5, so this should be rejected with tecNO_LINE.
+        {
+            auto withdrawTx =
+                vault.withdraw({.depositor = depositor, .id = keylet.key, .amount = asset(10)});
+            withdrawTx[sfDestination] = charlie.human();
+            env(withdrawTx, ter{tecNO_LINE});
+            env.close();
+        }
+        auto const charlieBalanceBefore = env.balance(charlie, asset.raw().get<Issue>());
+
+        // Withdraw the equivalent amount in shares to charlie. This should also be rejected.<
+        {
+            auto withdrawTx = vault.withdraw(
+                {.depositor = depositor, .id = keylet.key, .amount = STAmount(share, 10'000'000)});
+            withdrawTx[sfDestination] = charlie.human();
+            env(withdrawTx, ter{tecNO_LINE});
+            env.close();
+
+            // Verify that charlie received IOU beyond their trustline limit
+            // (their limit is 5, but they now hold 10).
+            auto const charlieBalanceAfter = env.balance(charlie, asset.raw().get<Issue>());
+            BEAST_EXPECT(charlieBalanceAfter == charlieBalanceBefore);
+        }
+    }
+
 public:
     void
     run() override
@@ -5249,6 +5443,7 @@ public:
         testVaultClawbackBurnShares();
         testVaultClawbackAssets();
         testAssetsMaximum();
+        testBug6_LimitBypassWithShares();
     }
 };
 

src/xrpld/rpc/handlers/GetAggregatePrice.cpp

@@ -218,6 +218,12 @@ doGetAggregatePrice(RPC::JsonContext& context)
         return result;
     }
 
+    // Get the ledger
+    std::shared_ptr<ReadView const> ledger;
+    result = RPC::lookupLedger(ledger, context);
+    if (!ledger)
+        return result;  // LCOV_EXCL_LINE
+
     // Collect the dataset into bimap keyed by lastUpdateTime and
     // STAmount (Number is int64 and price is uint64)
     Prices prices;
@@ -238,11 +244,6 @@ doGetAggregatePrice(RPC::JsonContext& context)
             return result;
         }
 
-        std::shared_ptr<ReadView const> ledger;
-        result = RPC::lookupLedger(ledger, context);
-        if (!ledger)
-            return result;  // LCOV_EXCL_LINE
-
         auto const sle = ledger->read(keylet::oracle(*account, *documentID));
         iteratePriceData(context, sle, [&](STObject const& node) {
             auto const& series = node.getFieldArray(sfPriceDataSeries);
@@ -284,8 +285,8 @@ doGetAggregatePrice(RPC::JsonContext& context)
     if (auto const threshold = std::get<std::uint32_t>(timeThreshold))
     {
         // threshold defines an acceptable range {max,min} of lastUpdateTime as
-        // {latestTime, latestTime - threshold}, the prices with lastUpdateTime
-        // greater than (latestTime - threshold) are erased.
+        // {latestTime, latestTime - threshold}. Prices with lastUpdateTime
+        // less than (latestTime - threshold) are erased (outdated prices).
         auto const oldestTime = prices.left.rbegin()->first;
         auto const upperBound = latestTime > threshold ? (latestTime - threshold) : oldestTime;
         if (upperBound > oldestTime)