From f5af42a64089ab0563c343a9eba234627d8afe5e Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 18 Aug 2022 20:29:36 -0400 Subject: [PATCH 1/6] Remove use of deprecated std::iterator --- .../detail/aged_container_iterator.h | 59 ++++++++----------- .../container/detail/aged_ordered_container.h | 40 ++++++------- .../detail/aged_unordered_container.h | 42 ++++++------- src/ripple/beast/core/List.h | 3 +- src/ripple/beast/core/LockFreeStack.h | 15 +---- 5 files changed, 69 insertions(+), 90 deletions(-) diff --git a/src/ripple/beast/container/detail/aged_container_iterator.h b/src/ripple/beast/container/detail/aged_container_iterator.h index cd8677ce1..7467ad33c 100644 --- a/src/ripple/beast/container/detail/aged_container_iterator.h +++ b/src/ripple/beast/container/detail/aged_container_iterator.h @@ -30,23 +30,21 @@ class aged_ordered_container; namespace detail { -// Idea for Base template argument to prevent having to repeat -// the base class declaration comes from newbiz on ##c++/Freenode -// // If Iterator is SCARY then this iterator will be as well. -template < - bool is_const, - class Iterator, - class Base = std::iterator< - typename std::iterator_traits::iterator_category, - typename std::conditional< - is_const, - typename Iterator::value_type::stashed::value_type const, - typename Iterator::value_type::stashed::value_type>::type, - typename std::iterator_traits::difference_type>> -class aged_container_iterator : public Base +template +class aged_container_iterator { public: + using iterator_category = + typename std::iterator_traits::iterator_category; + using value_type = typename std::conditional< + is_const, + typename Iterator::value_type::stashed::value_type const, + typename Iterator::value_type::stashed::value_type>::type; + using difference_type = + typename std::iterator_traits::difference_type; + using pointer = value_type*; + using reference = value_type&; using time_point = typename Iterator::value_type::stashed::time_point; aged_container_iterator() = default; @@ -56,13 +54,11 @@ public: template < bool other_is_const, class OtherIterator, - class OtherBase, class = typename std::enable_if< (other_is_const == false || is_const == true) && std::is_same::value == false>::type> explicit aged_container_iterator( - aged_container_iterator const& - other) + aged_container_iterator const& other) : m_iter(other.m_iter) { } @@ -70,22 +66,19 @@ public: // Disable constructing a const_iterator from a non-const_iterator. template < bool other_is_const, - class OtherBase, class = typename std::enable_if< other_is_const == false || is_const == true>::type> aged_container_iterator( - aged_container_iterator const& - other) + aged_container_iterator const& other) : m_iter(other.m_iter) { } // Disable assigning a const_iterator to a non-const iterator - template + template auto operator=( - aged_container_iterator const& - other) -> + aged_container_iterator const& other) -> typename std::enable_if< other_is_const == false || is_const == true, aged_container_iterator&>::type @@ -94,20 +87,18 @@ public: return *this; } - template + template bool - operator==( - aged_container_iterator const& - other) const + operator==(aged_container_iterator const& + other) const { return m_iter == other.m_iter; } - template + template bool - operator!=( - aged_container_iterator const& - other) const + operator!=(aged_container_iterator const& + other) const { return m_iter != other.m_iter; } @@ -142,13 +133,13 @@ public: return prev; } - typename Base::reference + reference operator*() const { return m_iter->value; } - typename Base::pointer + pointer operator->() const { return &m_iter->value; @@ -167,7 +158,7 @@ private: template friend class aged_unordered_container; - template + template friend class aged_container_iterator; template diff --git a/src/ripple/beast/container/detail/aged_ordered_container.h b/src/ripple/beast/container/detail/aged_ordered_container.h index ed6585dd5..9da5f20a0 100644 --- a/src/ripple/beast/container/detail/aged_ordered_container.h +++ b/src/ripple/beast/container/detail/aged_ordered_container.h @@ -989,22 +989,20 @@ public: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> - beast::detail::aged_container_iterator - erase(beast::detail::aged_container_iterator pos); + beast::detail::aged_container_iterator + erase(beast::detail::aged_container_iterator pos); // enable_if prevents erase (reverse_iterator first, reverse_iterator last) // from compiling template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> - beast::detail::aged_container_iterator + beast::detail::aged_container_iterator erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last); + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last); template auto @@ -1019,10 +1017,9 @@ public: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> void - touch(beast::detail::aged_container_iterator pos) + touch(beast::detail::aged_container_iterator pos) { touch(pos, clock().now()); } @@ -1264,11 +1261,10 @@ private: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> void touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now); template < @@ -2010,13 +2006,13 @@ template < class Clock, class Compare, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_ordered_container:: - erase(beast::detail::aged_container_iterator pos) + erase(beast::detail::aged_container_iterator pos) { unlink_and_delete_element(&*((pos++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( pos.iterator()); } @@ -2028,17 +2024,17 @@ template < class Clock, class Compare, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_ordered_container:: erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last) + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last) { for (; first != last;) unlink_and_delete_element(&*((first++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( first.iterator()); } @@ -2173,11 +2169,11 @@ template < class Clock, class Compare, class Allocator> -template +template void aged_ordered_container:: touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now) { auto& e(*pos.iterator()); diff --git a/src/ripple/beast/container/detail/aged_unordered_container.h b/src/ripple/beast/container/detail/aged_unordered_container.h index 8bc2330fa..920e6196b 100644 --- a/src/ripple/beast/container/detail/aged_unordered_container.h +++ b/src/ripple/beast/container/detail/aged_unordered_container.h @@ -1205,15 +1205,15 @@ public: return emplace(std::forward(args)...); } - template - beast::detail::aged_container_iterator - erase(beast::detail::aged_container_iterator pos); + template + beast::detail::aged_container_iterator + erase(beast::detail::aged_container_iterator pos); - template - beast::detail::aged_container_iterator + template + beast::detail::aged_container_iterator erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last); + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last); template auto @@ -1222,9 +1222,9 @@ public: void swap(aged_unordered_container& other) noexcept; - template + template void - touch(beast::detail::aged_container_iterator pos) + touch(beast::detail::aged_container_iterator pos) { touch(pos, clock().now()); } @@ -1541,10 +1541,10 @@ private: insert_unchecked(first, last); } - template + template void touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now) { auto& e(*pos.iterator()); @@ -3044,8 +3044,8 @@ template < class Hash, class KeyEqual, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_unordered_container< IsMulti, IsMap, @@ -3054,11 +3054,11 @@ aged_unordered_container< Clock, Hash, KeyEqual, - Allocator>:: - erase(beast::detail::aged_container_iterator pos) + Allocator>::erase(beast::detail::aged_container_iterator + pos) { unlink_and_delete_element(&*((pos++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( pos.iterator()); } @@ -3071,8 +3071,8 @@ template < class Hash, class KeyEqual, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_unordered_container< IsMulti, IsMap, @@ -3083,13 +3083,13 @@ aged_unordered_container< KeyEqual, Allocator>:: erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last) + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last) { for (; first != last;) unlink_and_delete_element(&*((first++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( first.iterator()); } diff --git a/src/ripple/beast/core/List.h b/src/ripple/beast/core/List.h index 1daf5cda7..9b3c889d6 100644 --- a/src/ripple/beast/core/List.h +++ b/src/ripple/beast/core/List.h @@ -72,11 +72,12 @@ private: template class ListIterator - : public std::iterator { public: + using iterator_category = std::bidirectional_iterator_tag; using value_type = typename beast::detail::CopyConst::type; + using difference_type = std::ptrdiff_t; using pointer = value_type*; using reference = value_type&; using size_type = std::size_t; diff --git a/src/ripple/beast/core/LockFreeStack.h b/src/ripple/beast/core/LockFreeStack.h index ff022b96a..107564415 100644 --- a/src/ripple/beast/core/LockFreeStack.h +++ b/src/ripple/beast/core/LockFreeStack.h @@ -29,18 +29,7 @@ namespace beast { //------------------------------------------------------------------------------ template -class LockFreeStackIterator : public std::iterator< - std::forward_iterator_tag, - typename Container::value_type, - typename Container::difference_type, - typename std::conditional< - IsConst, - typename Container::const_pointer, - typename Container::pointer>::type, - typename std::conditional< - IsConst, - typename Container::const_reference, - typename Container::reference>::type> +class LockFreeStackIterator { protected: using Node = typename Container::Node; @@ -48,7 +37,9 @@ protected: typename std::conditional::type; public: + using iterator_category = std::forward_iterator_tag; using value_type = typename Container::value_type; + using difference_type = typename Container::difference_type; using pointer = typename std::conditional< IsConst, typename Container::const_pointer, From e40e38e8d3ce447668215fd8dfb37a1a2b5504e9 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Tue, 13 Sep 2022 10:16:31 -0700 Subject: [PATCH 2/6] Introduce fixRemoveNFTokenAutoTrustLine amendment: It turns out that the feature enabled by the tfTrustLine flag on an NFTokenMint transaction could be used as a means to attack the NFToken issuer. Details are in https://github.com/XRPLF/rippled/issues/4300 The fixRemoveNFTokenAutoTrustLine amendment removes the ability to set the tfTrustLine flag on an NFTokenMint transaction. Closes 4300. --- src/ripple/app/tx/impl/NFTokenMint.cpp | 18 +- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/TxFlags.h | 18 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/NFToken_test.cpp | 287 +++++++++++++------------ 5 files changed, 192 insertions(+), 135 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index b4e391c3e..f4d3eb856 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -40,7 +40,23 @@ NFTokenMint::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (ctx.tx.getFlags() & tfNFTokenMintMask) + // Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between + // accounts allowed a TrustLine to be added to the issuer of that token + // without explicit permission from that issuer. This was enabled by + // minting the NFToken with the tfTrustLine flag set. + // + // That capability could be used to attack the NFToken issuer. It + // would be possible for two accounts to trade the NFToken back and forth + // building up any number of TrustLines on the issuer, increasing the + // issuer's reserve without bound. + // + // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the + // tfTrustLine flag as a way to prevent the attack. But until the + // amendment passes we still need to keep the old behavior available. + std::uint32_t const NFTokenMintMask = + ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine) ? tfNFTokenMintMask + : tfNFTokenMintOldMask; + if (ctx.tx.getFlags() & NFTokenMintMask) return temINVALID_FLAG; if (auto const f = ctx.tx[~sfTransferFee]) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index f0d0c8efb..ffee01e05 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,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 = 50; +static constexpr std::size_t numFeatures = 51; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -338,6 +338,7 @@ extern uint256 const featureExpandedSignerList; extern uint256 const fixNFTokenDirV1; extern uint256 const fixNFTokenNegOffer; extern uint256 const featureNonFungibleTokensV1_1; +extern uint256 const fixRemoveNFTokenAutoTrustLine; } // namespace ripple diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 0b907c722..0ad088c41 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -120,9 +120,25 @@ constexpr std::uint32_t const tfOnlyXRP = 0x00000002; constexpr std::uint32_t const tfTrustLine = 0x00000004; constexpr std::uint32_t const tfTransferable = 0x00000008; -constexpr std::uint32_t const tfNFTokenMintMask = +// Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between +// accounts allowed a TrustLine to be added to the issuer of that token +// without explicit permission from that issuer. This was enabled by +// minting the NFToken with the tfTrustLine flag set. +// +// That capability could be used to attack the NFToken issuer. It +// would be possible for two accounts to trade the NFToken back and forth +// building up any number of TrustLines on the issuer, increasing the +// issuer's reserve without bound. +// +// The fixRemoveNFTokenAutoTrustLine amendment disables minting with the +// tfTrustLine flag as a way to prevent the attack. But until the +// amendment passes we still need to keep the old behavior available. +constexpr std::uint32_t const tfNFTokenMintOldMask = ~(tfUniversal | tfBurnable | tfOnlyXRP | tfTrustLine | tfTransferable); +constexpr std::uint32_t const tfNFTokenMintMask = + ~(tfUniversal | tfBurnable | tfOnlyXRP | tfTransferable); + // NFTokenCreateOffer flags: constexpr std::uint32_t const tfSellNFToken = 0x00000001; constexpr std::uint32_t const tfNFTokenCreateOfferMask = diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index fcd774ce9..f8f2fa5c7 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -447,6 +447,7 @@ REGISTER_FEATURE(ExpandedSignerList, Supported::yes, DefaultVote::no) REGISTER_FIX (fixNFTokenDirV1, Supported::yes, DefaultVote::no); REGISTER_FIX (fixNFTokenNegOffer, Supported::yes, DefaultVote::no); REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 2db31afe0..ac86bf749 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -1574,7 +1574,6 @@ class NFToken_test : public beast::unit_test::suite using namespace test::jtx; - Env env{*this, features}; Account const alice{"alice"}; Account const becky{"becky"}; Account const cheri{"cheri"}; @@ -1583,155 +1582,179 @@ class NFToken_test : public beast::unit_test::suite IOU const gwCAD(gw["CAD"]); IOU const gwEUR(gw["EUR"]); - env.fund(XRP(1000), alice, becky, cheri, gw); - env.close(); - - // Set trust lines so becky and cheri can use gw's currency. - env(trust(becky, gwAUD(1000))); - env(trust(cheri, gwAUD(1000))); - env(trust(becky, gwCAD(1000))); - env(trust(cheri, gwCAD(1000))); - env(trust(becky, gwEUR(1000))); - env(trust(cheri, gwEUR(1000))); - env.close(); - env(pay(gw, becky, gwAUD(500))); - env(pay(gw, becky, gwCAD(500))); - env(pay(gw, becky, gwEUR(500))); - env(pay(gw, cheri, gwAUD(500))); - env(pay(gw, cheri, gwCAD(500))); - env.close(); - - // An nft without flagCreateTrustLines but with a non-zero transfer - // fee will not allow creating offers that use IOUs for payment. - for (std::uint32_t xferFee : {0, 1}) + // The behavior of this test changes dramatically based on the + // presence (or absence) of the fixRemoveNFTokenAutoTrustLine + // amendment. So we test both cases here. + for (auto const& tweakedFeatures : + {features - fixRemoveNFTokenAutoTrustLine, + features | fixRemoveNFTokenAutoTrustLine}) { - uint256 const nftNoAutoTrustID{ - token::getNextID(env, alice, 0u, tfTransferable, xferFee)}; - env(token::mint(alice, 0u), - token::xferFee(xferFee), - txflags(tfTransferable)); + Env env{*this, tweakedFeatures}; + env.fund(XRP(1000), alice, becky, cheri, gw); env.close(); - // becky buys the nft for 1 drop. - uint256 const beckyBuyOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftNoAutoTrustID, drops(1)), - token::owner(alice)); + // Set trust lines so becky and cheri can use gw's currency. + env(trust(becky, gwAUD(1000))); + env(trust(cheri, gwAUD(1000))); + env(trust(becky, gwCAD(1000))); + env(trust(cheri, gwCAD(1000))); + env(trust(becky, gwEUR(1000))); + env(trust(cheri, gwEUR(1000))); env.close(); - env(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); + env(pay(gw, becky, gwAUD(500))); + env(pay(gw, becky, gwCAD(500))); + env(pay(gw, becky, gwEUR(500))); + env(pay(gw, cheri, gwAUD(500))); + env(pay(gw, cheri, gwCAD(500))); env.close(); - // becky attempts to sell the nft for AUD. - TER const createOfferTER = - xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS); - uint256 const beckyOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)), - txflags(tfSellNFToken), - ter(createOfferTER)); - env.close(); + // An nft without flagCreateTrustLines but with a non-zero transfer + // fee will not allow creating offers that use IOUs for payment. + for (std::uint32_t xferFee : {0, 1}) + { + uint256 const nftNoAutoTrustID{ + token::getNextID(env, alice, 0u, tfTransferable, xferFee)}; + env(token::mint(alice, 0u), + token::xferFee(xferFee), + txflags(tfTransferable)); + env.close(); - // cheri offers to buy the nft for CAD. - uint256 const cheriOfferIndex = - keylet::nftoffer(cheri, env.seq(cheri)).key; - env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)), - token::owner(becky), - ter(createOfferTER)); - env.close(); + // becky buys the nft for 1 drop. + uint256 const beckyBuyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, drops(1)), + token::owner(alice)); + env.close(); + env(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); + env.close(); - // To keep things tidy, cancel the offers. - env(token::cancelOffer(becky, {beckyOfferIndex})); - env(token::cancelOffer(cheri, {cheriOfferIndex})); - env.close(); - } - // An nft with flagCreateTrustLines but with a non-zero transfer - // fee allows transfers using IOUs for payment. - { - std::uint16_t transferFee = 10000; // 10% + // becky attempts to sell the nft for AUD. + TER const createOfferTER = + xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS); + uint256 const beckyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)), + txflags(tfSellNFToken), + ter(createOfferTER)); + env.close(); - uint256 const nftAutoTrustID{token::getNextID( - env, alice, 0u, tfTransferable | tfTrustLine, transferFee)}; - env(token::mint(alice, 0u), - token::xferFee(transferFee), - txflags(tfTransferable | tfTrustLine)); - env.close(); + // cheri offers to buy the nft for CAD. + uint256 const cheriOfferIndex = + keylet::nftoffer(cheri, env.seq(cheri)).key; + env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)), + token::owner(becky), + ter(createOfferTER)); + env.close(); - // becky buys the nft for 1 drop. - uint256 const beckyBuyOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftAutoTrustID, drops(1)), - token::owner(alice)); - env.close(); - env(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); - env.close(); + // To keep things tidy, cancel the offers. + env(token::cancelOffer(becky, {beckyOfferIndex})); + env(token::cancelOffer(cheri, {cheriOfferIndex})); + env.close(); + } + // An nft with flagCreateTrustLines but with a non-zero transfer + // fee allows transfers using IOUs for payment. + { + std::uint16_t transferFee = 10000; // 10% - // becky sells the nft for AUD. - uint256 const beckySellOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)), - txflags(tfSellNFToken)); - env.close(); - env(token::acceptSellOffer(cheri, beckySellOfferIndex)); - env.close(); + uint256 const nftAutoTrustID{token::getNextID( + env, alice, 0u, tfTransferable | tfTrustLine, transferFee)}; - // alice should now have a trust line for gwAUD. - BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10)); + // If the fixRemoveNFTokenAutoTrustLine amendment is active + // then this transaction fails. + { + TER const mintTER = + tweakedFeatures[fixRemoveNFTokenAutoTrustLine] + ? static_cast(temINVALID_FLAG) + : static_cast(tesSUCCESS); - // becky buys the nft back for CAD. - uint256 const beckyBuyBackOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftAutoTrustID, gwCAD(50)), - token::owner(cheri)); - env.close(); - env(token::acceptBuyOffer(cheri, beckyBuyBackOfferIndex)); - env.close(); + env(token::mint(alice, 0u), + token::xferFee(transferFee), + txflags(tfTransferable | tfTrustLine), + ter(mintTER)); + env.close(); - // alice should now have a trust line for gwAUD and gwCAD. - BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10)); - BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(5)); - } - // Now that alice has trust lines already established, an nft without - // flagCreateTrustLines will work for preestablished trust lines. - { - std::uint16_t transferFee = 5000; // 5% - uint256 const nftNoAutoTrustID{ - token::getNextID(env, alice, 0u, tfTransferable, transferFee)}; - env(token::mint(alice, 0u), - token::xferFee(transferFee), - txflags(tfTransferable)); - env.close(); + // If fixRemoveNFTokenAutoTrustLine is active the rest + // of this test falls on its face. + if (tweakedFeatures[fixRemoveNFTokenAutoTrustLine]) + break; + } + // becky buys the nft for 1 drop. + uint256 const beckyBuyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, drops(1)), + token::owner(alice)); + env.close(); + env(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); + env.close(); - // alice sells the nft using AUD. - uint256 const aliceSellOfferIndex = - keylet::nftoffer(alice, env.seq(alice)).key; - env(token::createOffer(alice, nftNoAutoTrustID, gwAUD(200)), - txflags(tfSellNFToken)); - env.close(); - env(token::acceptSellOffer(cheri, aliceSellOfferIndex)); - env.close(); + // becky sells the nft for AUD. + uint256 const beckySellOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)), + txflags(tfSellNFToken)); + env.close(); + env(token::acceptSellOffer(cheri, beckySellOfferIndex)); + env.close(); - // alice should now have AUD(210): - // o 200 for this sale and - // o 10 for the previous sale's fee. - BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(210)); + // alice should now have a trust line for gwAUD. + BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10)); - // cheri can't sell the NFT for EUR, but can for CAD. - env(token::createOffer(cheri, nftNoAutoTrustID, gwEUR(50)), - txflags(tfSellNFToken), - ter(tecNO_LINE)); - env.close(); - uint256 const cheriSellOfferIndex = - keylet::nftoffer(cheri, env.seq(cheri)).key; - env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)), - txflags(tfSellNFToken)); - env.close(); - env(token::acceptSellOffer(becky, cheriSellOfferIndex)); - env.close(); + // becky buys the nft back for CAD. + uint256 const beckyBuyBackOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, gwCAD(50)), + token::owner(cheri)); + env.close(); + env(token::acceptBuyOffer(cheri, beckyBuyBackOfferIndex)); + env.close(); - // alice should now have CAD(10): - // o 5 from this sale's fee and - // o 5 for the previous sale's fee. - BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(10)); + // alice should now have a trust line for gwAUD and gwCAD. + BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10)); + BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(5)); + } + // Now that alice has trust lines preestablished, an nft without + // flagCreateTrustLines will work for preestablished trust lines. + { + std::uint16_t transferFee = 5000; // 5% + uint256 const nftNoAutoTrustID{token::getNextID( + env, alice, 0u, tfTransferable, transferFee)}; + env(token::mint(alice, 0u), + token::xferFee(transferFee), + txflags(tfTransferable)); + env.close(); + + // alice sells the nft using AUD. + uint256 const aliceSellOfferIndex = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::createOffer(alice, nftNoAutoTrustID, gwAUD(200)), + txflags(tfSellNFToken)); + env.close(); + env(token::acceptSellOffer(cheri, aliceSellOfferIndex)); + env.close(); + + // alice should now have AUD(210): + // o 200 for this sale and + // o 10 for the previous sale's fee. + BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(210)); + + // cheri can't sell the NFT for EUR, but can for CAD. + env(token::createOffer(cheri, nftNoAutoTrustID, gwEUR(50)), + txflags(tfSellNFToken), + ter(tecNO_LINE)); + env.close(); + uint256 const cheriSellOfferIndex = + keylet::nftoffer(cheri, env.seq(cheri)).key; + env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)), + txflags(tfSellNFToken)); + env.close(); + env(token::acceptSellOffer(becky, cheriSellOfferIndex)); + env.close(); + + // alice should now have CAD(10): + // o 5 from this sale's fee and + // o 5 for the previous sale's fee. + BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(10)); + } } } From 9a31f321cd5be0f1c6ebcb83c8f681be96672a2e Mon Sep 17 00:00:00 2001 From: seelabs Date: Fri, 9 Sep 2022 14:34:48 -0400 Subject: [PATCH 3/6] Allow gcc 12 compilation: Compiling with gcc 12 on manjaro (arch variant) had compilation errors without adding an additional include file. --- src/ripple/basics/impl/Archive.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ripple/basics/impl/Archive.cpp b/src/ripple/basics/impl/Archive.cpp index 47e3d1ab3..73e14a936 100644 --- a/src/ripple/basics/impl/Archive.cpp +++ b/src/ripple/basics/impl/Archive.cpp @@ -23,6 +23,8 @@ #include #include +#include + namespace ripple { void From df66e4151e2a8402eeb3c4d2c035e3f7a608f045 Mon Sep 17 00:00:00 2001 From: Ikko Ashimine Date: Fri, 26 Aug 2022 02:11:39 +0900 Subject: [PATCH 4/6] Fix typo in detail/Node.h minumum -> minimum --- src/ripple/app/rdb/backend/detail/Node.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/rdb/backend/detail/Node.h b/src/ripple/app/rdb/backend/detail/Node.h index fa7e39c83..23f5bce20 100644 --- a/src/ripple/app/rdb/backend/detail/Node.h +++ b/src/ripple/app/rdb/backend/detail/Node.h @@ -110,7 +110,7 @@ std::size_t getRows(soci::session& session, TableType type); /** - * @brief getRowsMinMax Returns minumum ledger sequence, + * @brief getRowsMinMax Returns minimum ledger sequence, * maximum ledger sequence and total number of rows in given table. * @param session Session with database. * @param type Table ID for which the result is returned. From be1ce5eca970cd2d8d076deef8fe8a85ab378043 Mon Sep 17 00:00:00 2001 From: Michael Legleux Date: Fri, 19 Aug 2022 15:26:46 -0700 Subject: [PATCH 5/6] Pin postgres, zlib, krb5, libuv and cassandra to stable versions --- Builds/CMake/deps/Postgres.cmake | 2 +- Builds/CMake/deps/cassandra.cmake | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Builds/CMake/deps/Postgres.cmake b/Builds/CMake/deps/Postgres.cmake index 7ad7f169a..bb94832a4 100644 --- a/Builds/CMake/deps/Postgres.cmake +++ b/Builds/CMake/deps/Postgres.cmake @@ -13,7 +13,7 @@ if(reporting) ExternalProject_Add(postgres_src PREFIX ${nih_cache_path} GIT_REPOSITORY https://github.com/postgres/postgres.git - GIT_TAG master + GIT_TAG REL_14_5 CONFIGURE_COMMAND ./configure --without-readline > /dev/null BUILD_COMMAND ${CMAKE_COMMAND} -E env --unset=MAKELEVEL make UPDATE_COMMAND "" diff --git a/Builds/CMake/deps/cassandra.cmake b/Builds/CMake/deps/cassandra.cmake index 8f1e799dc..4563a3413 100644 --- a/Builds/CMake/deps/cassandra.cmake +++ b/Builds/CMake/deps/cassandra.cmake @@ -11,7 +11,7 @@ if(reporting) ExternalProject_Add(zlib_src PREFIX ${nih_cache_path} GIT_REPOSITORY https://github.com/madler/zlib.git - GIT_TAG master + GIT_TAG v1.2.12 INSTALL_COMMAND "" BUILD_BYPRODUCTS /${ep_lib_prefix}z.a LOG_BUILD TRUE @@ -45,7 +45,7 @@ if(reporting) ExternalProject_Add(krb5_src PREFIX ${nih_cache_path} GIT_REPOSITORY https://github.com/krb5/krb5.git - GIT_TAG master + GIT_TAG krb5-1.20-final UPDATE_COMMAND "" CONFIGURE_COMMAND autoreconf src && CFLAGS=-fcommon ./src/configure --enable-static --disable-shared > /dev/null BUILD_IN_SOURCE 1 @@ -80,7 +80,7 @@ if(reporting) ExternalProject_Add(libuv_src PREFIX ${nih_cache_path} GIT_REPOSITORY https://github.com/libuv/libuv.git - GIT_TAG v1.x + GIT_TAG v1.44.2 INSTALL_COMMAND "" BUILD_BYPRODUCTS /${ep_lib_prefix}uv_a.a LOG_BUILD TRUE @@ -106,7 +106,7 @@ if(reporting) ExternalProject_Add(cassandra_src PREFIX ${nih_cache_path} GIT_REPOSITORY https://github.com/datastax/cpp-driver.git - GIT_TAG master + GIT_TAG 2.16.2 CMAKE_ARGS -DLIBUV_ROOT_DIR=${BINARY_DIR} -DLIBUV_LIBARY=${BINARY_DIR}/libuv_a.a From ba3c0e51455a88d76d90b996f20c0f102ac3f5a0 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 14 Sep 2022 15:52:05 -0700 Subject: [PATCH 6/6] Set version to 1.9.4 --- RELEASENOTES.md | 46 +++++++++++++++++++++++++- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5f5810fc6..4403110e0 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -7,6 +7,50 @@ This document contains the release notes for `rippled`, the reference server imp Have new ideas? Need help with setting up your node? Come visit us [here](https://github.com/xrplf/rippled/issues/new/choose) +# Introducing XRP Ledger version 1.9.4 + +Version 1.9.4 of `rippled`, the reference implementation of the XRP Ledger protocol is now available. This release introduces an amendment that removes the ability for an NFT issuer to indicate that trust lines should be automatically created for royalty payments from secondary sales of NFTs, in response to a bug report that indicated how this functionality could be abused to mount a denial of service attack against the issuer. + +## Action Required + +This release introduces a new amendment to the XRP Ledger protocol, **`fixRemoveNFTokenAutoTrustLine`** to mitigate a potential denial-of-service attack against NFT issuers that minted NFTs and allowed secondary trading of those NFTs to create trust lines for any asset. + +This amendment is open for voting according to the XRP Ledger's [amendment process](https://xrpl.org/amendments.html), which enables protocol changes following two weeks of >80% support from trusted validators. + +If you operate an XRP Ledger server, then you should upgrade to version 1.9.4 within two weeks, to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. + +For more information about NFTs on the XRP Ledger, see [NFT Conceptual Overview](https://xrpl.org/nft-conceptual-overview.html). + + +## Install / Upgrade + +On supported platforms, see the [instructions on installing or updating `rippled`](https://xrpl.org/install-rippled.html). + +## Changelog + +## Contributions + +The primary change in this release is the following bug fix: + +- **Introduce fixRemoveNFTokenAutoTrustLine amendment**: Introduces the `fixRemoveNFTokenAutoTrustLine` amendment, which disables the `tfTrustLine` flag, which a malicious attacker could exploit to mount denial-of-service attacks against NFT issuers that specified the flag on their NFTs. ([#4301](https://github.com/XRPLF/rippled/4301)) + + +### GitHub + +The public source code repository for `rippled` is hosted on GitHub at . + +We welcome all contributions and invite everyone to join the community of XRP Ledger developers and help us build the Internet of Value. + +### Credits + +The following people contributed directly to this release: + +- Scott Schurr +- Howard Hinnant +- Scott Determan +- Ikko Ashimine + + # Introducing XRP Ledger version 1.9.3 Version 1.9.3 of `rippled`, the reference server implementation of the XRP Ledger protocol is now available. This release corrects minor technical flaws with the code that loads configured amendment votes after a startup and the copy constructor of `PublicKey`. @@ -19,7 +63,7 @@ On supported platforms, see the [instructions on installing or updating `rippled ## Contributions -This releases contains the following bug fixes: +This release contains the following bug fixes: - **Change by-value to by-reference to persist vote**: A minor technical flaw, caused by use of a copy instead of a reference, resulted in operator-configured "yes" votes to not be properly loaded after a restart. ([#4256](https://github.com/XRPLF/rippled/pull/4256)) - **Properly handle self-assignment of PublicKey**: The `PublicKey` copy assignment operator mishandled the case where a `PublicKey` would be assigned to itself, and could result in undefined behavior. diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 734dc11cb..9a28a55cb 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.9.3" +char const* const versionString = "1.9.4" // clang-format on #if defined(DEBUG) || defined(SANITIZER)