From d04d3d3c4ada2bb81a395cc1b59be1f9bbdb937c Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 25 May 2017 00:13:20 -0700 Subject: [PATCH] Implement additional invariant checks: Invariant checks run after a transaction has been processed and ensure that the end result of that transaction did not violate any protocol rules. New checks include: * XRP balance checks for negative balances * Offer balance checks for negative balances * Zero balance checks when handling escrow --- src/ripple/app/tx/impl/InvariantCheck.cpp | 130 ++++++++++++++++++++++ src/ripple/app/tx/impl/InvariantCheck.h | 75 ++++++++++++- 2 files changed, 200 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/tx/impl/InvariantCheck.cpp b/src/ripple/app/tx/impl/InvariantCheck.cpp index 03a15c576..840abea56 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.cpp +++ b/src/ripple/app/tx/impl/InvariantCheck.cpp @@ -82,6 +82,136 @@ XRPNotCreated::finalize(STTx const& tx, TER /*tec*/, beast::Journal const& j) //------------------------------------------------------------------------------ +void +XRPBalanceChecks::visitEntry( + uint256 const&, + bool, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + auto isBad = [](STAmount const& balance) + { + if (!balance.native()) + return true; + + auto const drops = balance.xrp().drops(); + + // Can't have more than the number of drops instantiated + // in the genesis ledger. + if (drops > SYSTEM_CURRENCY_START) + return true; + + // Can't have a negative balance (0 is OK) + if (drops < 0) + return true; + + return false; + }; + + if(before && before->getType() == ltACCOUNT_ROOT) + bad_ |= isBad ((*before)[sfBalance]); + + if(after && after->getType() == ltACCOUNT_ROOT) + bad_ |= isBad ((*after)[sfBalance]); +} + +bool +XRPBalanceChecks::finalize(STTx const&, TER, beast::Journal const& j) +{ + if (bad_) + { + JLOG(j.fatal()) << "Invariant failed: incorrect account XRP balance"; + return false; + } + + return true; +} + +//------------------------------------------------------------------------------ + +void +NoBadOffers::visitEntry( + uint256 const&, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + auto isBad = [](STAmount const& pays, STAmount const& gets) + { + // An offer should never be negative + if (pays < beast::zero) + return true; + + if (gets < beast::zero) + return true; + + // Can't have an XRP to XRP offer: + return pays.native() && gets.native(); + }; + + if(before && before->getType() == ltOFFER) + bad_ |= isBad ((*before)[sfTakerPays], (*before)[sfTakerGets]); + + if(after && after->getType() == ltOFFER) + bad_ |= isBad((*after)[sfTakerPays], (*after)[sfTakerGets]); +} + +bool +NoBadOffers::finalize(STTx const& tx, TER, beast::Journal const& j) +{ + if (bad_) + { + JLOG(j.fatal()) << "Invariant failed: offer with a bad amount"; + return false; + } + + return true; +} + +//------------------------------------------------------------------------------ + +void +NoZeroEscrow::visitEntry( + uint256 const&, + bool isDelete, + std::shared_ptr const& before, + std::shared_ptr const& after) +{ + auto isBad = [](STAmount const& amount) + { + if (!amount.native()) + return true; + + if (amount.xrp().drops() <= 0) + return true; + + if (amount.xrp().drops() >= SYSTEM_CURRENCY_START) + return true; + + return false; + }; + + if(before && before->getType() == ltESCROW) + bad_ |= isBad((*before)[sfAmount]); + + if(after && after->getType() == ltESCROW) + bad_ |= isBad((*after)[sfAmount]); +} + +bool +NoZeroEscrow::finalize(STTx const& tx, TER, beast::Journal const& j) +{ + if (bad_) + { + JLOG(j.fatal()) << "Invariant failed: escrow specifies invalid amount"; + return false; + } + + return true; +} + +//------------------------------------------------------------------------------ + void AccountRootsNotDeleted::visitEntry( uint256 const&, diff --git a/src/ripple/app/tx/impl/InvariantCheck.h b/src/ripple/app/tx/impl/InvariantCheck.h index 554bb9cf2..04cb19360 100644 --- a/src/ripple/app/tx/impl/InvariantCheck.h +++ b/src/ripple/app/tx/impl/InvariantCheck.h @@ -132,9 +132,28 @@ public: }; /** - * @brief Invariant: corresponding modified ledger entries should match in type and - * added entries should be a valid type. - * + * @brief Invariant: An account XRP balance must be in XRP and take a value + between 0 and SYSTEM_CURRENCY_START drops, inclusive. + */ +class XRPBalanceChecks +{ + bool bad_ = false; + +public: + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); +}; + +/** + * @brief Invariant: corresponding modified ledger entries should match in type + * and added entries should be a valid type. */ class LedgerEntryTypesMatch { @@ -156,7 +175,6 @@ public: /** * @brief Invariant: Trust lines using XRP are not allowed. - * */ class NoXRPTrustLines { @@ -175,13 +193,60 @@ public: finalize(STTx const&, TER, beast::Journal const&); }; +/** + * @brief Invariant: offers should be for non-negative amounts and must not + * be XRP to XRP. + */ +class NoBadOffers +{ + bool bad_ = false; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); + +}; + +/** + * @brief Invariant: an escrow entry must take a value between 0 and + * SYSTEM_CURRENCY_START drops exclusive. + */ +class NoZeroEscrow +{ + bool bad_ = false; + +public: + + void + visitEntry( + uint256 const&, + bool, + std::shared_ptr const&, + std::shared_ptr const&); + + bool + finalize(STTx const&, TER, beast::Journal const&); + +}; + // additional invariant checks can be declared above and then added to this // tuple using InvariantChecks = std::tuple< AccountRootsNotDeleted, LedgerEntryTypesMatch, + XRPBalanceChecks, XRPNotCreated, - NoXRPTrustLines + NoXRPTrustLines, + NoBadOffers, + NoZeroEscrow >; /**