Enforce non-standard currency rule of not starting with 0x00

* Add amendment fixNonStandardCurrency * Fail AMMCreate, CheckCreate, OfferCreate, TrustSet, and Payment transactions if non-standard currency starts with 0x00
gregtatcam · Sep 9, 2024 · 519b6eb · 519b6eb
1 parent 2f432e8
commit 519b6eb
Show file tree

Hide file tree

Showing 14 changed files with 197 additions and 1 deletion.
diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h
@@ -80,7 +80,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 79;
+static constexpr std::size_t numFeatures = 80;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -372,6 +372,7 @@ extern uint256 const fixEnforceNFTokenTrustline;
 extern uint256 const fixInnerObjTemplate2;
 extern uint256 const featureInvariantsV1_1;
 extern uint256 const fixNFTokenPageLinks;
+extern uint256 const fixNonStandardCurrency;
 
 }  // namespace ripple
 

diff --git a/include/xrpl/protocol/UintTypes.h b/include/xrpl/protocol/UintTypes.h
@@ -71,6 +71,9 @@ noCurrency();
 Currency const&
 badCurrency();
 
+bool
+validCurrency(Currency const&);
+
 inline bool
 isXRP(Currency const& c)
 {

diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp
@@ -498,6 +498,7 @@ REGISTER_FIX    (fixReducedOffersV2,            Supported::yes, VoteBehavior::De
 REGISTER_FIX    (fixEnforceNFTokenTrustline,    Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixInnerObjTemplate2,          Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixNFTokenPageLinks,           Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixNonStandardCurrency,        Supported::yes, VoteBehavior::DefaultNo);
 // InvariantsV1_1 will be changes to Supported::yes when all the
 // invariants expected to be included under it are complete.
 REGISTER_FEATURE(InvariantsV1_1,                Supported::no, VoteBehavior::DefaultNo);

diff --git a/src/libxrpl/protocol/UintTypes.cpp b/src/libxrpl/protocol/UintTypes.cpp
@@ -132,4 +132,21 @@ badCurrency()
     return currency;
 }
 
+bool
+validCurrency(Currency const& currency)
+{
+    static constexpr Currency sIsoBits(
+        "FFFFFFFFFFFFFFFFFFFFFFFF000000FFFFFFFFFF");
+
+    // XRP or standard currency
+    if (currency == xrpCurrency() || ((currency & sIsoBits).isZero()))
+        return true;
+
+    // Non-standard currency must not start with 0x00
+    if (*currency.cbegin() != 0x00)
+        return true;
+
+    return false;
+}
+
 }  // namespace ripple
diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp
@@ -431,6 +431,26 @@ struct AMM_test : public jtx::AMMTest
             // Can't be cleared
             AMM amm2(env, gw, XRP(100), USD(100), ter(tecNO_PERMISSION));
         }
+
+        // Invalid non-standard currency
+        auto const invalid =
+            gw["0011111111111111111111111111111111111111"](100);
+        auto const valid = gw["0111111111111111111111111111111111111111"](100);
+        FeatureBitset const all{jtx::supported_amendments()};
+        for (auto const& features : {all, all - fixNonStandardCurrency})
+        {
+            Env env(*this, features);
+            env.fund(XRP(1'000), gw);
+            for (auto const& amt : {valid, invalid})
+            {
+                auto const err =
+                    features[fixNonStandardCurrency] && amt == invalid
+                    ? ter(temBAD_CURRENCY)
+                    : ter(tesSUCCESS);
+                AMM amm(env, gw, USD(100), amt, err);
+                AMM amm1(env, gw, amt, EUR(100), err);
+            }
+        }
     }
 
     void

diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp
@@ -2693,6 +2693,33 @@ class Check_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testInvalidNonStandardCurrency(FeatureBitset features)
+    {
+        testcase("Invalid Non-Standard Currency");
+        using namespace test::jtx;
+        auto const gw = Account("gw");
+        auto const alice = Account("alice");
+        auto const invalid =
+            gw["0011111111111111111111111111111111111111"](100);
+        auto const valid = gw["0111111111111111111111111111111111111111"](100);
+        auto const USD = gw["USD"];
+        for (auto const& features_ :
+             {features, features - fixNonStandardCurrency})
+        {
+            for (auto const& amt : {valid, invalid})
+            {
+                Env env(*this, features_);
+                env.fund(XRP(1'000), gw, alice);
+                auto const err =
+                    features_[fixNonStandardCurrency] && amt == invalid
+                    ? ter(temBAD_CURRENCY)
+                    : ter(tesSUCCESS);
+                env(check::create(gw, alice, amt), err);
+            }
+        }
+    }
+
     void
     testWithFeats(FeatureBitset features)
     {
@@ -2709,6 +2736,7 @@ class Check_test : public beast::unit_test::suite
         testCancelInvalid(features);
         testFix1623Enable(features);
         testWithTickets(features);
+        testInvalidNonStandardCurrency(features);
     }
 
 public:

diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp
@@ -1404,6 +1404,35 @@ struct Flow_test : public beast::unit_test::suite
         env.require(balance(alice, XRP(9000) - drops(20)));
     }
 
+    void
+    testInvalidNonStandardCurrency(FeatureBitset features)
+    {
+        testcase("Invalid Non-Standard Currency");
+        using namespace test::jtx;
+        auto const gw = Account("gw");
+        auto const alice = Account("alice");
+        auto const bob = Account("bob");
+        auto const invalid =
+            gw["0011111111111111111111111111111111111111"](100);
+        auto const valid = gw["0111111111111111111111111111111111111111"](100);
+        for (auto const& amt : {valid, invalid})
+        {
+            Env env(*this, features - fixNonStandardCurrency);
+            env.fund(XRP(1'000), gw, alice, bob);
+            env(offer(gw, XRP(100), amt));
+            env(trust(alice, amt));
+            env(trust(bob, amt));
+            env(pay(gw, alice, amt));
+            auto const err =
+                amt == invalid ? ter(temBAD_CURRENCY) : ter(tesSUCCESS);
+            env.enableFeature(fixNonStandardCurrency);
+            env(pay(alice, bob, amt),
+                sendmax(XRP(10)),
+                txflags(tfPartialPayment),
+                err);
+        }
+    }
+
     void
     testWithFeats(FeatureBitset features)
     {
@@ -1427,6 +1456,7 @@ struct Flow_test : public beast::unit_test::suite
         testReexecuteDirectStep(features);
         testSelfPayLowQualityOffer(features);
         testTicketPay(features);
+        testInvalidNonStandardCurrency(features);
     }
 
     void

diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp
@@ -5299,6 +5299,33 @@ class OfferBaseUtil_test : public beast::unit_test::suite
             env.balance(taker, XRP) == takerXRPBalance);
     }
 
+    void
+    testInvalidNonStandardCurrency(FeatureBitset features)
+    {
+        testcase("Invalid Non-Standard Currency");
+        using namespace jtx;
+        auto const gw = Account("gw");
+        auto const invalid =
+            gw["0011111111111111111111111111111111111111"](100);
+        auto const valid = gw["0111111111111111111111111111111111111111"](100);
+        auto const USD = gw["USD"];
+        for (auto const& features_ :
+             {features, features - fixNonStandardCurrency})
+        {
+            for (auto const& amt : {valid, invalid})
+            {
+                Env env(*this, features_);
+                env.fund(XRP(1'000), gw);
+                auto const err =
+                    features_[fixNonStandardCurrency] && amt == invalid
+                    ? ter(temBAD_CURRENCY)
+                    : ter(tesSUCCESS);
+                env(offer(gw, amt, USD(100)), err);
+                env(offer(gw, USD(100), amt), err);
+            }
+        }
+    }
+
     void
     testAll(FeatureBitset features)
     {
@@ -5361,6 +5388,7 @@ class OfferBaseUtil_test : public beast::unit_test::suite
         testRmSmallIncreasedQOffersXRP(features);
         testRmSmallIncreasedQOffersIOU(features);
         testFillOrKill(features);
+        testInvalidNonStandardCurrency(features);
     }
 
     void

diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp
@@ -618,6 +618,33 @@ class SetTrust_test : public beast::unit_test::suite
         env.close();
     }
 
+    void
+    testInvalidNonStandardCurrency(FeatureBitset features)
+    {
+        testcase("Invalid Non-Standard Currency");
+        using namespace test::jtx;
+        auto const gw = Account("gw");
+        auto const alice = Account("alice");
+        auto const invalid =
+            gw["0011111111111111111111111111111111111111"](100);
+        auto const valid = gw["0111111111111111111111111111111111111111"](100);
+        auto const USD = gw["USD"];
+        for (auto const& features_ :
+             {features, features - fixNonStandardCurrency})
+        {
+            for (auto const& amt : {valid, invalid})
+            {
+                Env env(*this, features_);
+                env.fund(XRP(1'000), gw, alice);
+                auto const err =
+                    features_[fixNonStandardCurrency] && amt == invalid
+                    ? ter(temBAD_CURRENCY)
+                    : ter(tesSUCCESS);
+                env(trust(alice, amt), err);
+            }
+        }
+    }
+
     void
     testWithFeats(FeatureBitset features)
     {
@@ -639,6 +666,7 @@ class SetTrust_test : public beast::unit_test::suite
         testExceedTrustLineLimit();
         testAuthFlagTrustLines();
         testTrustLineLimitsWithRippling();
+        testInvalidNonStandardCurrency(features);
     }
 
 public:

diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp
@@ -75,6 +75,14 @@ AMMCreate::preflight(PreflightContext const& ctx)
         return temBAD_FEE;
     }
 
+    if (ctx.rules.enabled(fixNonStandardCurrency) &&
+        (!validCurrency(amount.getCurrency()) ||
+         !validCurrency(amount2.getCurrency())))
+    {
+        JLOG(ctx.j.debug()) << "AMM Instance: bad non-standard currency";
+        return temBAD_CURRENCY;
+    }
+
     return preflight2(ctx);
 }
 

diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp
@@ -65,6 +65,14 @@ CreateCheck::preflight(PreflightContext const& ctx)
             JLOG(ctx.j.warn()) << "Malformed transaction: Bad currency.";
             return temBAD_CURRENCY;
         }
+
+        if (ctx.rules.enabled(fixNonStandardCurrency) &&
+            !validCurrency(sendMax.getCurrency()))
+        {
+            JLOG(ctx.j.debug())
+                << "Malformed transaction: bad non-standard currency";
+            return temBAD_CURRENCY;
+        }
     }
 
     if (auto const optExpiry = ctx.tx[~sfExpiration])

diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp
@@ -115,6 +115,13 @@ CreateOffer::preflight(PreflightContext const& ctx)
         return temBAD_CURRENCY;
     }
 
+    if (ctx.rules.enabled(fixNonStandardCurrency) &&
+        (!validCurrency(uPaysCurrency) || !validCurrency(uGetsCurrency)))
+    {
+        JLOG(j.debug()) << "Malformed offer: bad non-standard currency";
+        return temBAD_CURRENCY;
+    }
+
     if (saTakerPays.native() != !uPaysIssuerID ||
         saTakerGets.native() != !uGetsIssuerID)
     {

diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp
@@ -118,6 +118,15 @@ Payment::preflight(PreflightContext const& ctx)
                         << "Bad currency.";
         return temBAD_CURRENCY;
     }
+
+    if (ctx.rules.enabled(fixNonStandardCurrency) &&
+        (!validCurrency(uSrcCurrency) || !validCurrency(uDstCurrency)))
+    {
+        JLOG(j.trace()) << "Malformed transaction: "
+                        << "Bad non-standard currency.";
+        return temBAD_CURRENCY;
+    }
+
     if (account == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths)
     {
         // You're signing yourself a payment.

diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp
@@ -63,6 +63,14 @@ SetTrust::preflight(PreflightContext const& ctx)
         return temBAD_CURRENCY;
     }
 
+    if (ctx.rules.enabled(fixNonStandardCurrency) &&
+        !validCurrency(saLimitAmount.getCurrency()))
+    {
+        JLOG(ctx.j.debug())
+            << "Malformed transaction: bad non-standard currency";
+        return temBAD_CURRENCY;
+    }
+
     if (saLimitAmount < beast::zero)
     {
         JLOG(j.trace()) << "Malformed transaction: Negative credit limit.";