Merge master (1.7.3) into develop (1.8.0-b4)

RELEASENOTES.md

@@ -13,6 +13,20 @@ Have new ideas? Need help with setting up your node? Come visit us [here](https:
 
 # Releases
 
+## Version 1.7.3
+
+This is the 1.7.3 release of `rippled`, the reference implementation of the XRP Ledger protocol. This release addresses an OOB memory read identified by Guido Vranken, as well as an unrelated issue identified by the Ripple C++ team that could result in incorrect use of SLEs. Additionally, this version also introduces the `NegativeUNL` amendment, which corresponds to the feature which was introduced with the 1.6.0 release.
+
+## Action Required
+
+If you operate an XRP Ledger server, then you should upgrade to version 1.7.3 at your earliest convenience to mitigate the issues addressed in this hotfix. If a sufficient majority of servers on the network upgrade, the `NegativeUNL` amendment may gain a majority, at which point a two week activation countdown will begin. If the `NegativeUNL` amendment activates, servers running versions of `rippled` prior to 1.7.3 will become [amendment blocked](https://xrpl.org/amendments.html#amendment-blocked).
+
+### Bug Fixes
+
+- **Improve SLE usage in check cashing**: Fixes a situation which could result in the incorrect use of SLEs.
+- **Address OOB in base58 decoder**: Corrects a technical flaw that could allow an out-of-bounds memory read in the Base58 decoder.
+- **Add `NegativeUNL` as a supported amendment**: Introduces an amendment for the Negative UNL feature introduced in `rippled` 1.6.0.
+
 ## Version 1.7.2
 
 This the 1.7.2 release of rippled, the reference server implementation of the XRP Ledger protocol. This release protects against the security issue [CVE-2021-3499](https://www.openssl.org/news/secadv/20210325.txt) affecting OpenSSL, adds an amendment to fix an issue with small offers not being properly removed from order books in some cases, and includes various other minor fixes.

src/ripple/app/tx/impl/CashCheck.cpp

@@ -265,7 +265,7 @@ CashCheck::doApply()
     // directly on a View.
     PaymentSandbox psb(&ctx_.view());
 
-    auto const sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID]));
+    auto sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID]));
     if (!sleCheck)
     {
         JLOG(j_.fatal()) << "Precheck did not verify check's existence.";
@@ -273,10 +273,8 @@ CashCheck::doApply()
     }
 
     AccountID const srcId{sleCheck->getAccountID(sfAccount)};
-    auto const sleSrc = psb.peek(keylet::account(srcId));
+    if (!psb.exists(keylet::account(srcId)) ||
-    auto const sleDst = psb.peek(keylet::account(account_));
+        !psb.exists(keylet::account(account_)))
-
-    if (!sleSrc || !sleDst)
     {
         JLOG(ctx_.journal.fatal())
             << "Precheck did not verify source or destination's existence.";
@@ -295,7 +293,7 @@ CashCheck::doApply()
     // work to do...
     auto viewJ = ctx_.app.journal("View");
     auto const optDeliverMin = ctx_.tx[~sfDeliverMin];
-    bool const doFix1623{ctx_.view().rules().enabled(fix1623)};
+    bool const doFix1623{psb.rules().enabled(fix1623)};
 
     if (srcId != account_)
     {
@@ -304,7 +302,7 @@ CashCheck::doApply()
         // Flow() doesn't do XRP to XRP transfers.
         if (sendMax.native())
         {
-            // Here we need to calculate the amount of XRP sleSrc can send.
+            // Here we need to calculate the amount of XRP src can send.
             // The amount they have available is their balance minus their
             // reserve.
             //
@@ -376,6 +374,8 @@ CashCheck::doApply()
                 //     a. this (destination) account and
                 //     b. issuing account (not sending account).
 
+                auto const sleDst = psb.peek(keylet::account(account_));
+
                 // Can the account cover the trust line's reserve?
                 if (std::uint32_t const ownerCount = {sleDst->at(sfOwnerCount)};
                     mPriorBalance < psb.fees().accountReserve(ownerCount + 1))
@@ -412,6 +412,8 @@ CashCheck::doApply()
                 }
                 // clang-format on
 
+                psb.update(sleDst);
+
                 // Note that we _don't_ need to be careful about destroying
                 // the trust line if the check cashing fails.  The transaction
                 // machinery will automatically clean it up.
@@ -478,37 +480,42 @@ CashCheck::doApply()
                     // Set the delivered_amount metadata.
                     ctx_.deliver(result.actualAmountOut);
             }
+
             // Set the delivered amount metadata in all cases, not just
             // for DeliverMin.
             if (checkCashMakesTrustLine)
                 ctx_.deliver(result.actualAmountOut);
+
+            sleCheck = psb.peek(keylet::check(ctx_.tx[sfCheckID]));
         }
     }
 
     // Check was cashed.  If not a self send (and it shouldn't be), remove
     // check link from destination directory.
-    if (srcId != account_)
+    if (srcId != account_ &&
+        !psb.dirRemove(
+            keylet::ownerDir(account_),
+            sleCheck->at(sfDestinationNode),
+            sleCheck->key(),
+            true))
     {
-        std::uint64_t const page = {sleCheck->at(sfDestinationNode)};
+        JLOG(j_.fatal()) << "Unable to delete check from destination.";
-        if (!ctx_.view().dirRemove(
+        return tefBAD_LEDGER;
-                keylet::ownerDir(account_), page, sleCheck->key(), true))
-        {
-            JLOG(j_.fatal()) << "Unable to delete check from destination.";
-            return tefBAD_LEDGER;
-        }
     }
+
     // Remove check from check owner's directory.
+    if (!psb.dirRemove(
+            keylet::ownerDir(srcId),
+            sleCheck->at(sfOwnerNode),
+            sleCheck->key(),
+            true))
     {
-        std::uint64_t const page = {sleCheck->at(sfOwnerNode)};
+        JLOG(j_.fatal()) << "Unable to delete check from owner.";
-        if (!ctx_.view().dirRemove(
+        return tefBAD_LEDGER;
-                keylet::ownerDir(srcId), page, sleCheck->key(), true))
-        {
-            JLOG(j_.fatal()) << "Unable to delete check from owner.";
-            return tefBAD_LEDGER;
-        }
     }
+
     // If we succeeded, update the check owner's reserve.
-    adjustOwnerCount(psb, sleSrc, -1, viewJ);
+    adjustOwnerCount(psb, psb.peek(keylet::account(srcId)), -1, viewJ);
 
     // Remove check from ledger.
     psb.erase(sleCheck);

src/ripple/protocol/impl/Feature.cpp

@@ -131,7 +131,7 @@ detail::supportedAmendments()
         "fix1781",
         "HardenedValidations",
         "fixAmendmentMajorityCalc",
-        //"NegativeUNL",      // Commented out to prevent automatic enablement
+        "NegativeUNL",
         "TicketBatch",
         "FlowSortStrands",
         "fixSTAmountCanonicalize",

src/ripple/protocol/impl/tokens.cpp

@@ -149,7 +149,7 @@ encodeBase58(
 static std::string
 decodeBase58(std::string const& s)
 {
-    auto psz = s.c_str();
+    auto psz = reinterpret_cast<unsigned char const*>(s.c_str());
     auto remain = s.size();
     // Skip and count leading zeroes
     int zeroes = 0;