From be70d81bd7cd57f9a0978e5d0ec37afac97d2bb2 Mon Sep 17 00:00:00 2001
From: JoelKatz <DavidJoelSchwartz@GMail.com>
Date: Thu, 11 Oct 2018 13:56:59 -0700
Subject: [PATCH] Perform some extra checks on ledger changes

Perform some extra checks on the close time and sequence number
of a candidate for network consensus ledger. This tightens
defenses against some "insane/hostile supermajority" attacks.
---
 src/ripple/app/ledger/LedgerMaster.h        |  6 ++
 src/ripple/app/ledger/impl/LedgerMaster.cpp | 70 ++++++++++++++++++++-
 src/ripple/app/misc/NetworkOPs.cpp          |  6 +-
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/src/ripple/app/ledger/LedgerMaster.h b/src/ripple/app/ledger/LedgerMaster.h
index 50f67befa8..4f90c082e4 100644
--- a/src/ripple/app/ledger/LedgerMaster.h
+++ b/src/ripple/app/ledger/LedgerMaster.h
@@ -118,6 +118,12 @@ public:
         std::shared_ptr<Ledger const> const& ledger,
             bool isSynchronous, bool isCurrent);
 
+    /** Check the sequence number and parent close time of a
+        ledger against our clock and last validated ledger to
+        see if it can be the network's current ledger
+    */
+    bool canBeCurrent (std::shared_ptr<Ledger const> const& ledger);
+
     void switchLCL (std::shared_ptr<Ledger const> const& lastClosed);
 
     void failedSave(std::uint32_t seq, uint256 const& hash);
diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp
index cdcd227844..5be97b47db 100644
--- a/src/ripple/app/ledger/impl/LedgerMaster.cpp
+++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp
@@ -253,6 +253,72 @@ LedgerMaster::addHeldTransaction (
     mHeldTransactions.insert (transaction->getSTransaction ());
 }
 
+// Validate a ledger's close time and sequence number if we're considering
+// jumping to that ledger. This helps defend agains some rare hostile or
+// insane majority scenarios.
+bool
+LedgerMaster::canBeCurrent (std::shared_ptr<Ledger const> const& ledger)
+{
+    assert (ledger);
+
+    // Never jump to a candidate ledger that precedes our
+    // last validated ledger
+
+    auto validLedger = getValidatedLedger();
+    if (validLedger &&
+        (ledger->info().seq < validLedger->info().seq))
+    {
+        JLOG (m_journal.trace()) << "Candidate for current ledger has low seq "
+            << ledger->info().seq << " < " << validLedger->info().seq;
+        return false;
+    }
+
+    // Ensure this ledger's parent close time is within five minutes of
+    // our current time. If we already have a known fully-valid ledger
+    // we perform this check. Otherwise, we only do it if we've built a
+    // few ledgers as our clock can be off when we first start up
+
+    auto closeTime = app_.timeKeeper().closeTime();
+    auto ledgerClose = ledger->info().parentCloseTime;
+
+    if ((validLedger || (ledger->info().seq > 10)) &&
+        ((std::max (closeTime, ledgerClose) - std::min (closeTime, ledgerClose))
+            > 5min))
+    {
+        JLOG (m_journal.warn()) << "Candidate for current ledger has close time "
+            << to_string(ledgerClose) << " at network time "
+            << to_string(closeTime) << " seq " << ledger->info().seq;
+        return false;
+    }
+
+    if (validLedger)
+    {
+        // Sequence number must not be too high. We allow ten ledgers
+        // for time inaccuracies plus a maximum run rate of one ledger
+        // every two seconds. The goal is to prevent a malicious ledger
+        // from increasing our sequence unreasonably high
+
+        LedgerIndex maxSeq = validLedger->info().seq + 10;
+
+        if (closeTime > validLedger->info().parentCloseTime)
+            maxSeq += std::chrono::duration_cast<std::chrono::seconds>(
+                closeTime - validLedger->info().parentCloseTime).count() / 2;
+
+        if (ledger->info().seq > maxSeq)
+        {
+            JLOG (m_journal.warn()) << "Candidate for current ledger has high seq "
+                << ledger->info().seq << " > " << maxSeq;
+            return false;
+        }
+
+        JLOG (m_journal.trace()) << "Acceptable seq range: " <<
+            validLedger->info().seq << " <= " <<
+            ledger->info().seq << " <= " << maxSeq;
+    }
+
+    return true;
+}
+
 void
 LedgerMaster::switchLCL(std::shared_ptr<Ledger const> const& lastClosed)
 {
@@ -772,7 +838,9 @@ void
 LedgerMaster::checkAccept (
     std::shared_ptr<Ledger const> const& ledger)
 {
-    if (ledger->info().seq <= mValidLedgerSeq)
+    // Can we accept this ledger as our new last fully-validated ledger
+
+    if (! canBeCurrent (ledger))
         return;
 
     // Can we advance the last fully-validated ledger? If so, can we
diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp
index 1b28d18a97..2fb3cfe2f9 100644
--- a/src/ripple/app/misc/NetworkOPs.cpp
+++ b/src/ripple/app/misc/NetworkOPs.cpp
@@ -1330,10 +1330,12 @@ bool NetworkOPsImp::checkLastClosedLedger (
             closedLedger, 0, InboundLedger::Reason::CONSENSUS);
 
     if (consensus &&
-        !m_ledgerMaster.isCompatible(
-            *consensus, m_journal.debug(), "Not switching"))
+        (!m_ledgerMaster.canBeCurrent (consensus) ||
+            !m_ledgerMaster.isCompatible(
+                *consensus, m_journal.debug(), "Not switching")))
     {
         // Don't switch to a ledger not on the validated chain
+        // or with an invalid close time or sequence
         networkClosed = ourClosed->info().hash;
         return false;
     }