Merge pull request #5665

Author: Jeff Garzik

src/qt/paymentserver.cpp

@@ -509,12 +509,7 @@ bool PaymentServer::readPaymentRequestFromFile(const QString& filename, PaymentR
     }
 
     // BIP70 DoS protection
-    if (f.size() > BIP70_MAX_PAYMENTREQUEST_SIZE) {
-        qWarning() << QString("PaymentServer::%1: Payment request %2 is too large (%3 bytes, allowed %4 bytes).")
-            .arg(__func__)
-            .arg(filename)
-            .arg(f.size())
-            .arg(BIP70_MAX_PAYMENTREQUEST_SIZE);
+    if (!verifySize(f.size())) {
         return false;
     }
 
@@ -685,14 +680,13 @@ void PaymentServer::netRequestFinished(QNetworkReply* reply)
     reply->deleteLater();
 
     // BIP70 DoS protection
-    if (reply->size() > BIP70_MAX_PAYMENTREQUEST_SIZE) {
-        QString msg = tr("Payment request %1 is too large (%2 bytes, allowed %3 bytes).")
-            .arg(reply->request().url().toString())
-            .arg(reply->size())
-            .arg(BIP70_MAX_PAYMENTREQUEST_SIZE);
-
-        qWarning() << QString("PaymentServer::%1:").arg(__func__) << msg;
-        Q_EMIT message(tr("Payment request DoS protection"), msg, CClientUIInterface::MSG_ERROR);
+    if (!verifySize(reply->size())) {
+        Q_EMIT message(tr("Payment request rejected"),
+            tr("Payment request %1 is too large (%2 bytes, allowed %3 bytes).")
+                .arg(reply->request().url().toString())
+                .arg(reply->size())
+                .arg(BIP70_MAX_PAYMENTREQUEST_SIZE),
+            CClientUIInterface::MSG_ERROR);
         return;
     }
 
@@ -790,6 +784,18 @@ bool PaymentServer::verifyExpired(const payments::PaymentDetails& requestDetails
     return fVerified;
 }
 
+bool PaymentServer::verifySize(qint64 requestSize)
+{
+    bool fVerified = (requestSize <= BIP70_MAX_PAYMENTREQUEST_SIZE);
+    if (!fVerified) {
+        qWarning() << QString("PaymentServer::%1: Payment request too large (%2 bytes, allowed %3 bytes).")
+            .arg(__func__)
+            .arg(requestSize)
+            .arg(BIP70_MAX_PAYMENTREQUEST_SIZE);
+    }
+    return fVerified;
+}
+
 bool PaymentServer::verifyAmount(const CAmount& requestAmount)
 {
     bool fVerified = MoneyRange(requestAmount);

src/qt/paymentserver.h

@@ -88,13 +88,12 @@ class PaymentServer : public QObject
     // OptionsModel is used for getting proxy settings and display unit
     void setOptionsModel(OptionsModel *optionsModel);
 
-    // This is now public, because we use it in paymentservertests.cpp
-    static bool readPaymentRequestFromFile(const QString& filename, PaymentRequestPlus& request);
-
     // Verify that the payment request network matches the client network
     static bool verifyNetwork(const payments::PaymentDetails& requestDetails);
     // Verify if the payment request is expired
     static bool verifyExpired(const payments::PaymentDetails& requestDetails);
+    // Verify the payment request size is valid as per BIP70
+    static bool verifySize(qint64 requestSize);
     // Verify the payment request amount is valid
     static bool verifyAmount(const CAmount& requestAmount);
 
@@ -131,6 +130,7 @@ private Q_SLOTS:
     bool eventFilter(QObject *object, QEvent *event);
 
 private:
+    static bool readPaymentRequestFromFile(const QString& filename, PaymentRequestPlus& request);
     bool processPaymentRequest(const PaymentRequestPlus& request, SendCoinsRecipient& recipient);
     void fetchRequest(const QUrl& url);
 

src/qt/test/paymentservertests.cpp

@@ -185,7 +185,8 @@ void PaymentServerTests::paymentServerTests()
     tempFile.open();
     tempFile.write((const char*)randData, sizeof(randData));
     tempFile.close();
-    QCOMPARE(PaymentServer::readPaymentRequestFromFile(tempFile.fileName(), r.paymentRequest), false);
+    // compares 50001 <= BIP70_MAX_PAYMENTREQUEST_SIZE == false
+    QCOMPARE(PaymentServer::verifySize(tempFile.size()), false);
 
     // Payment request with amount overflow (amount is set to 21000001 BTC):
     data = DecodeBase64(paymentrequest5_cert2_BASE64);