[Qt] prevent amount overflow problem with payment requests

Bitcoin amounts are stored as uint64 in the protobuf messages (see
paymentrequest.proto), but CAmount is defined as int64_t. Because
of that we need to verify that single and accumulated amounts are
in a valid range and no variable overflow has happened.

- fixes #5624 (#5622)

Thanks @SergioDemianLerner for reporting that issue and also supplying us
with a possible solution.

- add static verifyAmount() function to PaymentServer and move the logging
  on error into the function
- also add a unit test to paymentservertests.cpp

1 files changed, 25 insertions, 0 deletions

diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp
index a00916bf7f..9aab944f6b 100644
--- a/src/qt/paymentserver.cpp
+++ b/src/qt/paymentserver.cpp
@@ -569,6 +569,14 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
             return false;
         }
 
+        // Bitcoin amounts are stored as (optional) uint64 in the protobuf messages (see paymentrequest.proto),
+        // but CAmount is defined as int64_t. Because of that we need to verify that amounts are in a valid range
+        // and no overflow has happened.
+        if (!verifyAmount(sendingTo.second)) {
+            emit message(tr("Payment request rejected"), tr("Invalid payment request."), CClientUIInterface::MSG_ERROR);
+            return false;
+        }
+
         // Extract and check amounts
         CTxOut txOut(sendingTo.second, sendingTo.first);
         if (txOut.IsDust(::minRelayTxFee)) {
@@ -580,6 +588,11 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
         }
 
         recipient.amount += sendingTo.second;
+        // Also verify that the final amount is still in a valid range after adding additional amounts.
+        if (!verifyAmount(recipient.amount)) {
+            emit message(tr("Payment request rejected"), tr("Invalid payment request."), CClientUIInterface::MSG_ERROR);
+            return false;
+        }
     }
     // Store addresses and format them to fit nicely into the GUI
     recipient.address = addresses.join("<br />");
@@ -768,3 +781,15 @@ bool PaymentServer::verifyExpired(const payments::PaymentDetails& requestDetails
     }
     return fVerified;
 }
+
+bool PaymentServer::verifyAmount(const CAmount& requestAmount)
+{
+    bool fVerified = MoneyRange(requestAmount);
+    if (!fVerified) {
+        qWarning() << QString("PaymentServer::%1: Payment request amount out of allowed range (%2, allowed 0 - %3).")
+            .arg(__func__)
+            .arg(requestAmount)
+            .arg(MAX_MONEY);
+    }
+    return fVerified;
+}