From 35f4698aed5dce02f771b34cfbb690495cb5efcc Mon Sep 17 00:00:00 2001
From: seelabs <scott.determan@yahoo.com>
Date: Fri, 24 Feb 2017 11:16:12 -0500
Subject: [PATCH] Check for malformed public key on payment channel

---
 src/ripple/app/tx/impl/PayChan.cpp          |  5 ++
 src/ripple/rpc/handlers/AccountChannels.cpp |  9 ++--
 src/test/app/PayChan_test.cpp               | 59 +++++++++++++++++++++
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp
index 7180c7301c..4aeb509dd9 100644
--- a/src/ripple/app/tx/impl/PayChan.cpp
+++ b/src/ripple/app/tx/impl/PayChan.cpp
@@ -171,6 +171,9 @@ PayChanCreate::preflight (PreflightContext const& ctx)
     if (ctx.tx[sfAccount] == ctx.tx[sfDestination])
         return temDST_IS_SRC;
 
+    if (!publicKeyType(ctx.tx[sfPublicKey]))
+        return temMALFORMED;
+
     return preflight2 (ctx);
 }
 
@@ -378,6 +381,8 @@ PayChanClaim::preflight (PreflightContext const& ctx)
             return tecNO_PERMISSION;
 
         Keylet const k (ltPAYCHAN, ctx.tx[sfPayChannel]);
+        if (!publicKeyType(ctx.tx[sfPublicKey]))
+            return temMALFORMED;
         PublicKey const pk (ctx.tx[sfPublicKey]);
         Serializer msg;
         serializePayChanAuthorization (msg, k.key, authAmt);
diff --git a/src/ripple/rpc/handlers/AccountChannels.cpp b/src/ripple/rpc/handlers/AccountChannels.cpp
index 251b9d29f0..3af2b7e225 100644
--- a/src/ripple/rpc/handlers/AccountChannels.cpp
+++ b/src/ripple/rpc/handlers/AccountChannels.cpp
@@ -40,9 +40,12 @@ void addChannel (Json::Value& jsonLines, SLE const& line)
     jDst[jss::destination_account] = to_string (line[sfDestination]);
     jDst[jss::amount] = line[sfAmount].getText ();
     jDst[jss::balance] = line[sfBalance].getText ();
-    PublicKey const pk (line[sfPublicKey]);
-    jDst[jss::public_key] = toBase58 (TokenType::TOKEN_ACCOUNT_PUBLIC, pk);
-    jDst[jss::public_key_hex] = strHex (pk);
+    if (publicKeyType(line[sfPublicKey]))
+    {
+        PublicKey const pk (line[sfPublicKey]);
+        jDst[jss::public_key] = toBase58 (TokenType::TOKEN_ACCOUNT_PUBLIC, pk);
+        jDst[jss::public_key_hex] = strHex (pk);
+    }
     jDst[jss::settle_delay] = line[sfSettleDelay];
     if (auto const& v = line[~sfExpiration])
         jDst[jss::expiration] = *v;
diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp
index 23f4444f10..cb2b7f70bb 100644
--- a/src/test/app/PayChan_test.cpp
+++ b/src/test/app/PayChan_test.cpp
@@ -739,6 +739,13 @@ struct PayChan_test : public beast::unit_test::suite
             BEAST_EXPECT (r[jss::result][jss::channels][0u][jss::channel_id] == chan1Str);
             chan1PkStr = r[jss::result][jss::channels][0u][jss::public_key].asString();
         }
+        {
+            auto const r =
+                env.rpc ("account_channels", alice.human ());
+            BEAST_EXPECT (r[jss::result][jss::channels].size () == 1);
+            BEAST_EXPECT (r[jss::result][jss::channels][0u][jss::channel_id] == chan1Str);
+            chan1PkStr = r[jss::result][jss::channels][0u][jss::public_key].asString();
+        }
         {
             auto const r =
                 env.rpc ("account_channels", bob.human (), alice.human ());
@@ -818,6 +825,57 @@ struct PayChan_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testMalformedPK ()
+    {
+        testcase ("malformed pk");
+        using namespace jtx;
+        using namespace std::literals::chrono_literals;
+        Env env (*this, features (featurePayChan));
+        auto const alice = Account ("alice");
+        auto const bob = Account ("bob");
+        auto USDA = alice["USD"];
+        env.fund (XRP (10000), alice, bob);
+        auto const pk = alice.pk ();
+        auto const settleDelay = 100s;
+
+        auto jv = create (alice, bob, XRP (1000), settleDelay, pk);
+        auto const pkHex = strHex (pk.slice ());
+        jv["PublicKey"] = pkHex.substr(2, pkHex.size()-2);
+        env (jv, ter(temMALFORMED));
+        jv["PublicKey"] = pkHex.substr(0, pkHex.size()-2);
+        env (jv, ter(temMALFORMED));
+        auto badPrefix = pkHex;
+        badPrefix[0]='f';
+        badPrefix[1]='f';
+        jv["PublicKey"] = badPrefix;
+        env (jv, ter(temMALFORMED));
+
+        jv["PublicKey"] = pkHex;
+        env (jv);
+        auto const chan = channel (*env.current (), alice, bob);
+
+        auto const authAmt = XRP (100);
+        auto const sig = signClaimAuth (alice.pk (), alice.sk (), chan, authAmt);
+        jv = claim(bob, chan, authAmt.value(), authAmt.value(), Slice(sig), alice.pk());
+        jv["PublicKey"] = pkHex.substr(2, pkHex.size()-2);
+        env (jv, ter(temMALFORMED));
+        jv["PublicKey"] = pkHex.substr(0, pkHex.size()-2);
+        env (jv, ter(temMALFORMED));
+        badPrefix = pkHex;
+        badPrefix[0]='f';
+        badPrefix[1]='f';
+        jv["PublicKey"] = badPrefix;
+        env (jv, ter(temMALFORMED));
+
+        // missing public key
+        jv.removeMember("PublicKey");
+        env (jv, ter(temMALFORMED));
+
+        jv["PublicKey"] = pkHex;
+        env (jv);
+    }
+
     void
     run () override
     {
@@ -832,6 +890,7 @@ struct PayChan_test : public beast::unit_test::suite
         testMultiple ();
         testRPC ();
         testOptionalFields ();
+        testMalformedPK ();
     }
 };