fix: Enforce aggregate MaximumAmount in multi-send MPT (#6644)

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

src/libxrpl/ledger/View.cpp

@@ -2669,51 +2669,82 @@ rippleSendMultiMPT(
     beast::Journal j,
     WaiveTransferFee waiveFee)
 {
-    // Safe to get MPT since rippleSendMultiMPT is only called by
-    // accountSendMultiMPT
     auto const& issuer = mptIssue.getIssuer();
 
     auto const sle = view.read(keylet::mptIssuance(mptIssue.getMptID()));
     if (!sle)
         return tecOBJECT_NOT_FOUND;
 
-    // These may diverge
+    // For the issuer-as-sender case, track the running total to validate
+    // against MaximumAmount. The read-only SLE (view.read) is not updated
+    // by rippleCreditMPT, so a per-iteration SLE read would be stale.
+    // Use uint64_t, not STAmount, to keep MaximumAmount comparisons in exact
+    // integer arithmetic. STAmount implicitly converts to Number, whose
+    // small-scale mantissa (~16 digits) can lose precision for values near
+    // maxMPTokenAmount (19 digits).
+    std::uint64_t totalSendAmount{0};
+    std::uint64_t const maximumAmount =
+        sle->at(~sfMaximumAmount).value_or(maxMPTokenAmount);
+    std::uint64_t const outstandingAmount =
+        sle->getFieldU64(sfOutstandingAmount);
+
+    // actual accumulates the total cost to the sender (includes transfer
+    // fees for third-party transit sends). takeFromSender accumulates only
+    // the transit portion that is debited to the issuer in bulk after the
+    // loop. They diverge when there are transfer fees.
     STAmount takeFromSender{mptIssue};
     actual = takeFromSender;
 
-    for (auto const& r : receivers)
+    for (auto const& [receiverID, amt] : receivers)
     {
-        auto const& receiverID = r.first;
-        STAmount amount{mptIssue, r.second};
+        STAmount amount{mptIssue, amt};
 
         if (amount < beast::zero)
-        {
             return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
 
-        /* If we aren't sending anything or if the sender is the same as the
-         * receiver then we don't need to do anything.
-         */
-        if (!amount || (senderID == receiverID))
+        if (!amount || senderID == receiverID)
             continue;
 
         if (senderID == issuer || receiverID == issuer)
         {
-            // if sender is issuer, check that the new OutstandingAmount will
-            // not exceed MaximumAmount
             if (senderID == issuer)
             {
                 XRPL_ASSERT_PARTS(
                     takeFromSender == beast::zero,
                     "rippler::rippleSendMultiMPT",
                     "sender == issuer, takeFromSender == zero");
-                auto const sendAmount = amount.mpt().value();
-                auto const maximumAmount =
-                    sle->at(~sfMaximumAmount).value_or(maxMPTokenAmount);
-                if (sendAmount > maximumAmount ||
-                    sle->getFieldU64(sfOutstandingAmount) >
-                        maximumAmount - sendAmount)
-                    return tecPATH_DRY;
+
+                std::uint64_t const sendAmount = amount.mpt().value();
+
+                if (view.rules().enabled(fixSecurity3_1_3))
+                {
+                    // Post-fixSecurity3_1_3: aggregate MaximumAmount
+                    // check. WARNING: the order of conditions is
+                    // critical — each guards the subtraction in the
+                    // next against unsigned underflow. Do not reorder.
+                    bool const exceedsMaximumAmount =
+                        // This send alone exceeds the max cap
+                        sendAmount > maximumAmount ||
+                        // The aggregate of all sends exceeds the max cap
+                        totalSendAmount > maximumAmount - sendAmount ||
+                        // Outstanding + aggregate exceeds the max cap
+                        outstandingAmount >
+                            maximumAmount - sendAmount - totalSendAmount;
+
+                    if (exceedsMaximumAmount)
+                        return tecPATH_DRY;
+                    totalSendAmount += sendAmount;
+                }
+                else
+                {
+                    // Pre-fixSecurity3_1_3: per-iteration MaximumAmount
+                    // check. Reads sfOutstandingAmount from a stale
+                    // view.read() snapshot — incorrect for multi-destination
+                    // sends but retained for ledger replay compatibility.
+                    if (sendAmount > maximumAmount ||
+                        outstandingAmount > maximumAmount - sendAmount)
+                        return tecPATH_DRY;
+                }
             }
 
             // Direct send: redeeming MPTs and/or sending own MPTs.
@@ -2721,8 +2752,8 @@ rippleSendMultiMPT(
                     rippleCreditMPT(view, senderID, receiverID, amount, j))
                 return ter;
             actual += amount;
-            // Do not add amount to takeFromSender, because rippleCreditMPT took
-            // it
+            // Do not add amount to takeFromSender, because rippleCreditMPT
+            // took it.
 
             continue;
         }

src/test/app/MPToken_test.cpp

@@ -3625,6 +3625,139 @@ class MPToken_test : public beast::unit_test::suite
         mptAlice.claw(alice, bob, 1, tecNO_PERMISSION);
     }
 
+    void
+    testMultiSendMaximumAmount(FeatureBitset features)
+    {
+        // Verify that rippleSendMultiMPT correctly enforces MaximumAmount
+        // when the issuer sends to multiple receivers. Pre-fixSecurity3_1_3,
+        // a stale view.read() snapshot caused per-iteration checks to miss
+        // aggregate overflows. Post-fix, a running total is used instead.
+        testcase("Multi-send MaximumAmount enforcement");
+
+        using namespace test::jtx;
+
+        Account const issuer("issuer");
+        Account const alice("alice");
+        Account const bob("bob");
+
+        std::uint64_t constexpr maxAmt = 150;
+        Env env{*this, features};
+
+        MPTTester mptt(env, issuer, {.holders = {alice, bob}});
+        mptt.create(
+            {.maxAmt = maxAmt, .ownerCount = 1, .flags = tfMPTCanTransfer});
+        mptt.authorize({.account = alice});
+        mptt.authorize({.account = bob});
+
+        Asset const asset{MPTIssue{mptt.issuanceID()}};
+
+        // Each test case creates a fresh ApplyView and calls
+        // accountSendMulti from the issuer to the given receivers.
+        auto const runTest = [&](MultiplePaymentDestinations const& receivers,
+                                 TER expectedTer,
+                                 std::optional<std::uint64_t>
+                                     expectedOutstanding,
+                                 std::string const& label) {
+            ApplyViewImpl av(&*env.current(), tapNONE);
+            auto const ter = accountSendMulti(
+                av, issuer.id(), asset, receivers, env.app().journal("View"));
+            BEAST_EXPECTS(ter == expectedTer, label);
+
+            // Only verify OutstandingAmount on success — on error the
+            // view may contain partial state and must be discarded.
+            if (expectedOutstanding)
+            {
+                auto const sle =
+                    av.peek(keylet::mptIssuance(mptt.issuanceID()));
+                if (!BEAST_EXPECT(sle))
+                    return;
+                BEAST_EXPECTS(
+                    sle->getFieldU64(sfOutstandingAmount) ==
+                        *expectedOutstanding,
+                    label);
+            }
+        };
+
+        using R = MultiplePaymentDestinations;
+
+        // Post-amendment: aggregate check with running total
+        runTest(
+            R{{alice.id(), 100}, {bob.id(), 100}},
+            tecPATH_DRY,
+            std::nullopt,
+            "aggregate exceeds max");
+
+        runTest(
+            R{{alice.id(), 75}, {bob.id(), 75}},
+            tesSUCCESS,
+            maxAmt,
+            "aggregate at boundary");
+
+        runTest(
+            R{{alice.id(), 50}, {bob.id(), 50}},
+            tesSUCCESS,
+            100,
+            "aggregate within limit");
+
+        runTest(
+            R{{alice.id(), 150}, {bob.id(), 0}},
+            tesSUCCESS,
+            maxAmt,
+            "one receiver at max, other zero");
+
+        runTest(
+            R{{alice.id(), 151}, {bob.id(), 0}},
+            tecPATH_DRY,
+            std::nullopt,
+            "one receiver exceeds max, other zero");
+
+        // Issue 50 tokens so outstandingAmount is nonzero, then verify
+        // the third condition: outstandingAmount > maximumAmount - sendAmount -
+        // totalSendAmount
+        mptt.pay(issuer, alice, 50);
+        env.close();
+
+        // maxAmt=150, outstanding=50, so 100 more available
+        runTest(
+            R{{alice.id(), 50}, {bob.id(), 50}},
+            tesSUCCESS,
+            maxAmt,
+            "nonzero outstanding, aggregate at boundary");
+
+        runTest(
+            R{{alice.id(), 50}, {bob.id(), 51}},
+            tecPATH_DRY,
+            std::nullopt,
+            "nonzero outstanding, aggregate exceeds max");
+
+        runTest(
+            R{{alice.id(), 100}, {bob.id(), 0}},
+            tesSUCCESS,
+            maxAmt,
+            "nonzero outstanding, single send at remaining capacity");
+
+        runTest(
+            R{{alice.id(), 101}, {bob.id(), 0}},
+            tecPATH_DRY,
+            std::nullopt,
+            "nonzero outstanding, single send exceeds remaining capacity");
+
+        // Pre-amendment: the stale per-iteration check allows each
+        // individual send (100 <= 150) even though the aggregate (200)
+        // exceeds MaximumAmount. Preserved for ledger replay.
+        {
+            // KNOWN BUG (pre-fixSecurity3_1_3): preserved for ledger replay
+            // only
+            env.disableFeature(fixSecurity3_1_3);
+            runTest(
+                R{{alice.id(), 100}, {bob.id(), 100}},
+                tesSUCCESS,
+                250,
+                "pre-amendment allows over-send");
+            env.enableFeature(fixSecurity3_1_3);
+        }
+    }
+
 public:
     void
     run() override
@@ -3632,6 +3765,7 @@ public:
         using namespace test::jtx;
         FeatureBitset const all{testable_amendments()};
 
+        testMultiSendMaximumAmount(all);
         // MPTokenIssuanceCreate
         testCreateValidation(all - featureSingleAssetVault);
         testCreateValidation(all - featurePermissionedDomains);