From 11590d39b9888403ead8354302e308eca139ba17 Mon Sep 17 00:00:00 2001
From: Alex Morcos <morcos@chaincode.com>
Date: Wed, 12 Jul 2017 14:42:57 -0400
Subject: [PATCH] Properly bound check conf_target in wallet RPC calls

---
 src/rpc/mining.cpp       | 10 ++++++++++
 src/rpc/mining.h         |  3 +++
 src/wallet/rpcwallet.cpp | 14 +++++---------
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 5dc468e111..b8c94d32ec 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -30,6 +30,16 @@
 
 #include <univalue.h>
 
+unsigned int ParseConfirmTarget(const UniValue& value)
+{
+    int target = value.get_int();
+    unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE);
+    if (target < 1 || (unsigned int)target > max_target) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target));
+    }
+    return (unsigned int)target;
+}
+
 /**
  * Return average network hashes per second based on the last 'lookup' blocks,
  * or from the last difficulty change if 'lookup' is nonpositive.
diff --git a/src/rpc/mining.h b/src/rpc/mining.h
index a148d851da..868d7002b5 100644
--- a/src/rpc/mining.h
+++ b/src/rpc/mining.h
@@ -12,4 +12,7 @@
 /** Generate blocks (mine) */
 UniValue generateBlocks(std::shared_ptr<CReserveScript> coinbaseScript, int nGenerate, uint64_t nMaxTries, bool keepScript);
 
+/** Check bounds on a command line confirm target */
+unsigned int ParseConfirmTarget(const UniValue& value);
+
 #endif
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index c636fa8114..f983a61a9f 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -460,7 +460,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request)
     }
 
     if (request.params.size() > 6 && !request.params[6].isNull()) {
-        coin_control.m_confirm_target = request.params[6].get_int();
+        coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]);
     }
 
     if (request.params.size() > 7 && !request.params[7].isNull()) {
@@ -981,7 +981,7 @@ UniValue sendmany(const JSONRPCRequest& request)
     }
 
     if (request.params.size() > 6 && !request.params[6].isNull()) {
-        coin_control.m_confirm_target = request.params[6].get_int();
+        coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]);
     }
 
     if (request.params.size() > 7 && !request.params[7].isNull()) {
@@ -2795,7 +2795,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
             coinControl.signalRbf = options["replaceable"].get_bool();
         }
         if (options.exists("conf_target")) {
-            coinControl.m_confirm_target = options["conf_target"].get_int();
+            coinControl.m_confirm_target = ParseConfirmTarget(options["conf_target"]);
         }
         if (options.exists("estimate_mode")) {
             if (!FeeModeFromString(options["estimate_mode"].get_str(), coinControl.m_fee_mode)) {
@@ -2917,12 +2917,8 @@ UniValue bumpfee(const JSONRPCRequest& request)
 
         if (options.exists("confTarget") && options.exists("totalFee")) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget and totalFee options should not both be set. Please provide either a confirmation target for fee estimation or an explicit total fee for the transaction.");
-        } else if (options.exists("confTarget")) {
-            int target = options["confTarget"].get_int();
-            if (target <= 0) { // FIXME: Check upper bound too
-                throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid confTarget (cannot be <= 0)");
-            }
-            coin_control.m_confirm_target = target;
+        } else if (options.exists("confTarget")) { // TODO: alias this to conf_target
+            coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"]);
         } else if (options.exists("totalFee")) {
             totalFee = options["totalFee"].get_int64();
             if (totalFee <= 0) {