From f32be2b28dfa638bc205b8eaab9c9f6d08bc1fa7 Mon Sep 17 00:00:00 2001
From: Vinnie Falco <vinnie.falco@gmail.com>
Date: Fri, 28 Aug 2015 12:14:04 -0700
Subject: [PATCH] Fix SusPay condition check in Finish:

On a SusPayFinish, a check is added to make sure that the
presented digest matches the digest in the SusPay ledger
entry. Another check is added to make Finish transactions
containing sfProof fields that are not 32 bytes malformed.

This includes regression unit tests.
---
 src/ripple/app/tests/SusPay_test.cpp | 30 ++++++++++++++++++++++++++--
 src/ripple/app/tx/impl/SusPay.cpp    |  7 +++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/ripple/app/tests/SusPay_test.cpp b/src/ripple/app/tests/SusPay_test.cpp
index be84c0a507..ed1f70cf84 100644
--- a/src/ripple/app/tests/SusPay_test.cpp
+++ b/src/ripple/app/tests/SusPay_test.cpp
@@ -106,11 +106,12 @@ struct SusPay_test : public beast::unit_test::suite
         return jv;
     }
 
+    template <class Proof>
     static
     Json::Value
     finish (jtx::Account const& account,
         jtx::Account const& from, std::uint32_t seq,
-            uint256 const& digest, uint256 const& preimage)
+            uint256 const& digest, Proof const& proof)
     {
         Json::Value jv;
         jv[jss::TransactionType] = "SuspendedPaymentFinish";
@@ -120,7 +121,7 @@ struct SusPay_test : public beast::unit_test::suite
         jv["OfferSequence"] = seq;
         jv["Method"] = 1;
         jv["Digest"] = to_string(digest);
-        jv["Proof"] = to_string(preimage);
+        jv["Proof"] = to_string(proof);
         return jv;
     }
 
@@ -304,6 +305,31 @@ struct SusPay_test : public beast::unit_test::suite
             expect((*env.le("alice"))[sfOwnerCount] == 1);
             env.require(balance("carol", XRP(5000)));
         }
+        {
+            Env env(*this);
+            auto T = [&env](NetClock::duration const& d)
+                { return env.clock.now() + d; };
+            env.fund(XRP(5000), "alice", "bob", "carol");
+            auto const c = cond("receipt");
+            auto const seq = env.seq("alice");
+            env(condpay("alice", "carol", XRP(1000), c.first, T(S{1})));
+            // wrong digest
+            auto const cx = cond("bad");
+            env(finish("bob", "alice", seq, cx.first, cx.second),           ter(tecNO_PERMISSION));
+        }
+        {
+            Env env(*this);
+            auto T = [&env](NetClock::duration const& d)
+                { return env.clock.now() + d; };
+            env.fund(XRP(5000), "alice", "bob", "carol");
+            auto const p = from_hex_text<uint128>(
+                "0102030405060708090A0B0C0D0E0F");
+            auto const d = digest(p);
+            auto const seq = env.seq("alice");
+            env(condpay("alice", "carol", XRP(1000), d, T(S{1})));
+            // bad digest size
+            env(finish("bob", "alice", seq, d, p),                          ter(temMALFORMED));
+        }
     }
 
     void
diff --git a/src/ripple/app/tx/impl/SusPay.cpp b/src/ripple/app/tx/impl/SusPay.cpp
index f3c4259aa3..87e0fdb230 100644
--- a/src/ripple/app/tx/impl/SusPay.cpp
+++ b/src/ripple/app/tx/impl/SusPay.cpp
@@ -272,6 +272,8 @@ SusPayFinish::preflight (PreflightContext const& ctx)
                 return temMALFORMED;
             if (! ctx.tx[~sfProof])
                 return temMALFORMED;
+            if (ctx.tx[~sfProof]->size() != 32)
+                return temMALFORMED;
             sha256_hasher h;
             using beast::hash_append;
             hash_append(h, ctx.tx[sfProof]);
@@ -324,6 +326,11 @@ SusPayFinish::doApply()
             ctx_.view().info().parentCloseTime)
         return tecNO_PERMISSION;
 
+    // Same digest?
+    if ((*slep)[~sfDigest] && (! ctx_.tx[~sfMethod] ||
+            (ctx_.tx[~sfDigest] != (*slep)[~sfDigest])))
+        return tecNO_PERMISSION;
+
     AccountID const account = (*slep)[sfAccount];
 
     // Remove SusPay from owner directory