diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a00d6b85c1b..e4295277288 100644 --- 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 index a0a8069f669..c9f00eb5c78 100644 --- a/include/xrpl/protocol/UintTypes.h +++ b/include/xrpl/protocol/UintTypes.h @@ -71,6 +71,13 @@ noCurrency(); Currency const& badCurrency(); +enum class PaymentTx : bool { Yes = true, No = false }; +/** Valid currency as described in + * https://xrpl.org/docs/references/protocol/data-types + */ +bool +validCurrency(Currency const&, PaymentTx paymentTx = PaymentTx::No); + inline bool isXRP(Currency const& c) { diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index 078369bf20c..e5bb4c9496c 100644 --- 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 index c57bcc153a3..c4cffaa8147 100644 --- a/src/libxrpl/protocol/UintTypes.cpp +++ b/src/libxrpl/protocol/UintTypes.cpp @@ -132,4 +132,44 @@ badCurrency() return currency; } +bool +validCurrency(Currency const& currency, PaymentTx paymentTx) +{ + // Allow payments for invalid non-standard currencies + // created pre fixNonStandardCurrency amendment + static std::set whiteList = { + Currency{"0000000000000000000000000000000078415059"}, + Currency{"00000000004150756E6B30310000000000000000"}, + Currency{"0000000000D9A928EFBCBEE297A1EFBCBE29DBB6"}, + Currency{"0000000000414C6F676F30330000000000000000"}, + Currency{"0000000000000000000000005852500000000000"}, + Currency{"000028E0B2A05FE0B2A029E2948CE288A9E29490"}, + Currency{"00000028E2989EEFBE9FE28880EFBE9F29E2989E"}, + Currency{"00000028E381A3E29795E280BFE2979529E381A3"}, + Currency{"000000000000005C6D2F5F283E5F3C295F5C6D2F"}, + Currency{"00000028E295AFC2B0E296A1C2B0EFBC89E295AF"}, + Currency{"0000000000000000000000005852527570656500"}, + Currency{"000000000000000000000000302E310000000000"}, + Currency{"0000000000E1839A28E0B2A05FE0B2A0E1839A29"}, + Currency{"0000000048617070794E6577596561725852504C"}, + Currency{"0000E29D9AE29688E29590E29590E29688E29D9A"}, + Currency{"000028E297A35FE297A229E2948CE288A9E29490"}, + Currency{"00000000CA95E0B2A0E0B2BFE1B4A5E0B2A0CA94"}, + Currency{"000000282D5F282D5F282D5F2D295F2D295F2D29"}, + Currency{"0000000000000000000000000000000078415049"}, + Currency{"00000000000028E295ADE0B2B05FE280A2CC8129"}}; + 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 paymentTx == PaymentTx::Yes && whiteList.contains(currency); +} + } // namespace ripple diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index ceddc019504..6db0f220f90 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -431,6 +431,28 @@ 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); + auto const invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](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, invalidWhiteListed}) + { + auto const err = features[fixNonStandardCurrency] && + (amt == invalid || amt == invalidWhiteListed) + ? 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 index 31b45abf43a..dad407bea82 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -2693,6 +2693,35 @@ 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 invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](100); + auto const USD = gw["USD"]; + for (auto const& features_ : + {features, features - fixNonStandardCurrency}) + { + for (auto const& amt : {valid, invalid, invalidWhiteListed}) + { + Env env(*this, features_); + env.fund(XRP(1'000), gw, alice); + auto const err = features_[fixNonStandardCurrency] && + (amt == invalid || amt == invalidWhiteListed) + ? ter(temBAD_CURRENCY) + : ter(tesSUCCESS); + env(check::create(gw, alice, amt), err); + } + } + } + void testWithFeats(FeatureBitset features) { @@ -2709,6 +2738,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 index 9d1257d16bf..620349b622e 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1404,6 +1404,37 @@ 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); + auto const invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](100); + for (auto const& amt : {valid, invalid, invalidWhiteListed}) + { + 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 +1458,7 @@ struct Flow_test : public beast::unit_test::suite testReexecuteDirectStep(features); testSelfPayLowQualityOffer(features); testTicketPay(features); + testInvalidNonStandardCurrency(features); } void diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 9c0e09d6711..077b1fd89d2 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -7744,6 +7744,61 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } } + void + testInvalidNonStandardCurrency(FeatureBitset features) + { + testcase("Invalid Non-Standard Currency"); + using namespace test::jtx; + Account const alice{"alice"}; + Account const buyer{"buyer"}; + Account const gw{"gw"}; + auto const invalid = + gw["0011111111111111111111111111111111111111"](1000); + auto const valid = gw["0111111111111111111111111111111111111111"](1000); + auto const invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](100); + auto const USD = gw["USD"]; + for (auto const& amt : {valid, invalid, invalidWhiteListed}) + { + auto features_ = features - fixNonStandardCurrency; + Env env{*this, features_}; + + env.fund(XRP(1'000), alice, buyer, gw); + env.close(); + + env(trust(buyer, amt)); + env(pay(gw, buyer, amt)); + env.close(); + env(trust(alice, amt)); + env(pay(gw, alice, amt)); + env.close(); + + env.enableFeature(fixNonStandardCurrency); + + auto const err = (amt == invalid || amt == invalidWhiteListed) + ? ter(temBAD_CURRENCY) + : ter(tesSUCCESS); + + uint256 nftId = token::getNextID(env, alice, 0, tfTransferable, 10); + env(token::mint(alice, 0u), + txflags(tfTransferable), + token::xferFee(10)); + env.close(); + + env(token::createOffer(buyer, nftId, amt), + token::owner(alice), + err); + env.close(); + + if (features_[featureNFTokenMintOffer]) + { + nftId = token::getNextID(env, alice, 0u); + env(token::mint(alice, 0u), token::amount(amt), err); + env.close(); + } + } + } + void testWithFeats(FeatureBitset features) { @@ -7781,6 +7836,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testFixNFTokenBuyerReserve(features); testUnaskedForAutoTrustline(features); testNFTIssuerIsIOUIssuer(features); + testInvalidNonStandardCurrency(features); } public: diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 2b4245a1ae4..580d21a3c83 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -5299,6 +5299,35 @@ 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 invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](100); + auto const USD = gw["USD"]; + for (auto const& features_ : + {features, features - fixNonStandardCurrency}) + { + for (auto const& amt : {valid, invalid, invalidWhiteListed}) + { + Env env(*this, features_); + env.fund(XRP(1'000), gw); + auto const err = features_[fixNonStandardCurrency] && + (amt == invalid || amt == invalidWhiteListed) + ? 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 +5390,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 index 57e18d712f8..14ed444e972 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -618,6 +618,35 @@ 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 invalidWhiteListed = + gw["0000000000000000000000000000000078415059"](100); + auto const USD = gw["USD"]; + for (auto const& features_ : + {features, features - fixNonStandardCurrency}) + { + for (auto const& amt : {valid, invalid, invalidWhiteListed}) + { + Env env(*this, features_); + env.fund(XRP(1'000), gw, alice); + auto const err = features_[fixNonStandardCurrency] && + (amt == invalid || amt == invalidWhiteListed) + ? ter(temBAD_CURRENCY) + : ter(tesSUCCESS); + env(trust(alice, amt), err); + } + } + } + void testWithFeats(FeatureBitset features) { @@ -639,6 +668,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 index 237e1afa240..c3692345973 100644 --- 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 index 3a278eed738..949498b5b92 100644 --- 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 index 2a5145594a1..2f9647a90d6 100644 --- 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/NFTokenUtils.cpp b/src/xrpld/app/tx/detail/NFTokenUtils.cpp index 61ff8e200b3..84909ef8b68 100644 --- a/src/xrpld/app/tx/detail/NFTokenUtils.cpp +++ b/src/xrpld/app/tx/detail/NFTokenUtils.cpp @@ -850,6 +850,11 @@ tokenOfferCreatePreflight( if (dest == acctID) return temMALFORMED; } + + if (rules.enabled(fixNonStandardCurrency) && + !validCurrency(amount.getCurrency())) + return temBAD_CURRENCY; + return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 309e9d4a498..5ed0be97593 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -118,6 +118,16 @@ Payment::preflight(PreflightContext const& ctx) << "Bad currency."; return temBAD_CURRENCY; } + + if (ctx.rules.enabled(fixNonStandardCurrency) && + (!validCurrency(uSrcCurrency, PaymentTx::Yes) || + !validCurrency(uDstCurrency, PaymentTx::Yes))) + { + 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 index 3a7fe9cca0d..b51e4a9ee51 100644 --- 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.";