aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWladimir J. van der Laan <laanwj@gmail.com>2015-01-29 12:47:13 +0100
committerWladimir J. van der Laan <laanwj@gmail.com>2015-01-29 12:48:27 +0100
commit7823598fa4774141568e5b6d73c4277a452f8d16 (patch)
tree97e3d2e86cee27ff8d77f93b706981841626dbfc
parent7620ef9e7edbc3526e71922e7b080b5a332d6897 (diff)
parent6715efb9ca5cabeb07ae4ba8390a6e1b7638f66c (diff)
Merge pull request #5620
6715efb [Qt] Payment request expiration bug fix (re-done) (Philip Kaufmann)
-rw-r--r--src/qt/paymentserver.cpp25
-rw-r--r--src/qt/paymentserver.h2
-rw-r--r--src/qt/sendcoinsdialog.cpp4
-rw-r--r--src/qt/test/paymentrequestdata.h72
-rw-r--r--src/qt/test/paymentservertests.cpp34
-rw-r--r--src/qt/walletmodel.cpp8
-rw-r--r--src/qt/walletmodel.h5
7 files changed, 137 insertions, 13 deletions
diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp
index d642fdae36..46ffd055a5 100644
--- a/src/qt/paymentserver.cpp
+++ b/src/qt/paymentserver.cpp
@@ -521,8 +521,6 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
return false;
if (request.IsInitialized()) {
- const payments::PaymentDetails& details = request.getDetails();
-
// Payment request network matches client network?
if (!verifyNetwork(request.getDetails())) {
emit message(tr("Payment request rejected"), tr("Payment request network doesn't match client network."),
@@ -531,16 +529,15 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins
return false;
}
- // Expired payment request?
- if (details.has_expires() && (int64_t)details.expires() < GetTime())
- {
- emit message(tr("Payment request rejected"), tr("Payment request has expired."),
+ // Make sure any payment requests involved are still valid.
+ // This is re-checked just before sending coins in WalletModel::sendCoins().
+ if (verifyExpired(request.getDetails())) {
+ emit message(tr("Payment request rejected"), tr("Payment request expired."),
CClientUIInterface::MSG_ERROR);
return false;
}
- }
- else {
+ } else {
emit message(tr("Payment request error"), tr("Payment request is not initialized."),
CClientUIInterface::MSG_ERROR);
@@ -759,3 +756,15 @@ bool PaymentServer::verifyNetwork(const payments::PaymentDetails& requestDetails
}
return fVerified;
}
+
+bool PaymentServer::verifyExpired(const payments::PaymentDetails& requestDetails)
+{
+ bool fVerified = (requestDetails.has_expires() && (int64_t)requestDetails.expires() < GetTime());
+ if (fVerified) {
+ const QString requestExpires = QString::fromStdString(DateTimeStrFormat("%Y-%m-%d %H:%M:%S", (int64_t)requestDetails.expires()));
+ qWarning() << QString("PaymentServer::%1: Payment request expired \"%2\".")
+ .arg(__func__)
+ .arg(requestExpires);
+ }
+ return fVerified;
+}
diff --git a/src/qt/paymentserver.h b/src/qt/paymentserver.h
index 9330e9a89c..db5f44ff1d 100644
--- a/src/qt/paymentserver.h
+++ b/src/qt/paymentserver.h
@@ -93,6 +93,8 @@ public:
// 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);
signals:
// Fired when a valid payment request is received
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 1f9bd57545..9e8743d805 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -529,6 +529,10 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn
case WalletModel::InsaneFee:
msgParams.first = tr("A fee higher than %1 is considered an insanely high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), 10000000));
break;
+ case WalletModel::PaymentRequestExpired:
+ msgParams.first = tr("Payment request expired!");
+ msgParams.second = CClientUIInterface::MSG_ERROR;
+ break;
// included to prevent a compiler warning.
case WalletModel::OK:
default:
diff --git a/src/qt/test/paymentrequestdata.h b/src/qt/test/paymentrequestdata.h
index 67c9a41e61..50636d7c67 100644
--- a/src/qt/test/paymentrequestdata.h
+++ b/src/qt/test/paymentrequestdata.h
@@ -361,3 +361,75 @@ gAFwThsozZxkZxzCn4R8WxNiLFV6m0ye9fEtSbolfaW+EjBMpO03lr/dwNnrclhg\
ew+A05xfZztrAt16XKEY7qKJ/eY2nLd0fVAIu/nIt+7/VYVXT83zLrWc150aRS7W\
AdJbL3JOJLs6Eyp5zrPbfI8faRttFAdONKDrJgIpuW1E3g==\
";
+
+//
+// Expired payment request (expires is set to 1 = 1970-01-01 00:00:01)
+//
+const char* paymentrequest2_cert2_BASE64 =
+"\
+Egt4NTA5K3NoYTI1NhrQBArNBDCCAkkwggExoAMCAQICAQEwDQYJKoZIhvcNAQEL\
+BQAwITEfMB0GA1UEAwwWUGF5bWVudFJlcXVlc3QgVGVzdCBDQTAeFw0xNTAxMTEx\
+ODIxMDhaFw0yNTAxMDgxODIxMDhaMCExHzAdBgNVBAMMFlBheW1lbnRSZXF1ZXN0\
+IFRlc3QgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMsZqzkzeBGo+i2N\
+mUak3Ciodr1V7S062VOy7N0OQYNDQHYkgDFAUET7cEb5VJaHPv5m3ppTBpU9xBcf\
+wbHHUt4VjA+mhRmYrl1khjvZM+X8kEqvWn20BtcM9R6r0yIYec8UERDDHBleL/P8\
+RkxEnVLjYTV9zigCXfMsgYb3EQShAgMBAAGjEDAOMAwGA1UdEwQFMAMBAf8wDQYJ\
+KoZIhvcNAQELBQADggEBABUJpl3QCqsoDSxAsQdV6zKT4VGV76AzoGj7etQsQY+r\
++S26VfWh/fMobEzuxFChr0USgLJ6FoK78hAtoZvt1lrye9yqFv/ig3WLWsJKWHHb\
+3RT6oR03CIwZXFSUasi08QDVLxafwsU5OMcPLucF3a1lRL1ccYrNgVCCx1+X7Bos\
+tIgDGRQQ4AyoHTcfVd2hEGeUv7k14mOxFsAp6851yosHq9Q2kwmdH+rHEJbjof87\
+yyKLagc4owyXBZYkQmkeHWCNqnuRmO5vUsfVb0UUrkD64o7Th/NjwooA7SCiUXl6\
+dfygT1b7ggpx7GC+sP2DsIM47IAZ55drjqX5u2f+Ba0iQgoEdGVzdBIgCICt4gQS\
+GXapFASsapRTBKxoykO9YhoackY1CqLyiKwYiNLUpQUgASoQVGVzdGluZyB0ZXN0\
+bmV0ISqAATXq9A5nmJgtmee/bQTeHeif4w1YYFPBlKghwx6qbVgXTWnwBJtOQhhV\
+sZdzbTl95ENR7/Y7VJupW9kDWobCK7zUUhLAzUlwmLlcx6itHw8LTUF5HK+AwsZm\
+Zs85lISGvOS0NZW/ENa6l+oQRnL87oqVZr/EDGiuqjz6T0ThQi0l\
+";
+
+//
+// Unexpired payment request (expires is set to 0x7FFFFFFFFFFFFFFF = max. int64_t)
+//
+const char* paymentrequest3_cert2_BASE64 =
+"\
+Egt4NTA5K3NoYTI1NhrQBArNBDCCAkkwggExoAMCAQICAQEwDQYJKoZIhvcNAQEL\
+BQAwITEfMB0GA1UEAwwWUGF5bWVudFJlcXVlc3QgVGVzdCBDQTAeFw0xNTAxMTEx\
+ODIxMDhaFw0yNTAxMDgxODIxMDhaMCExHzAdBgNVBAMMFlBheW1lbnRSZXF1ZXN0\
+IFRlc3QgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMsZqzkzeBGo+i2N\
+mUak3Ciodr1V7S062VOy7N0OQYNDQHYkgDFAUET7cEb5VJaHPv5m3ppTBpU9xBcf\
+wbHHUt4VjA+mhRmYrl1khjvZM+X8kEqvWn20BtcM9R6r0yIYec8UERDDHBleL/P8\
+RkxEnVLjYTV9zigCXfMsgYb3EQShAgMBAAGjEDAOMAwGA1UdEwQFMAMBAf8wDQYJ\
+KoZIhvcNAQELBQADggEBABUJpl3QCqsoDSxAsQdV6zKT4VGV76AzoGj7etQsQY+r\
++S26VfWh/fMobEzuxFChr0USgLJ6FoK78hAtoZvt1lrye9yqFv/ig3WLWsJKWHHb\
+3RT6oR03CIwZXFSUasi08QDVLxafwsU5OMcPLucF3a1lRL1ccYrNgVCCx1+X7Bos\
+tIgDGRQQ4AyoHTcfVd2hEGeUv7k14mOxFsAp6851yosHq9Q2kwmdH+rHEJbjof87\
+yyKLagc4owyXBZYkQmkeHWCNqnuRmO5vUsfVb0UUrkD64o7Th/NjwooA7SCiUXl6\
+dfygT1b7ggpx7GC+sP2DsIM47IAZ55drjqX5u2f+Ba0iSgoEdGVzdBIgCICt4gQS\
+GXapFASsapRTBKxoykO9YhoackY1CqLyiKwYyNfZpQUg//////////9/KhBUZXN0\
+aW5nIHRlc3RuZXQhKoABNwi8WnMW4aMvbmvorTiiWJLFhofLFnsoWCJnj3rWLnLh\
+n3w6q/fZ26p50ERL/noxdTUfeFsKnlECkUu/fOcOrqyYDiwvxI0SZ034DleVyFU1\
+Z3T+X0zcL8oe7bX01Yf+s2V+5JXQXarKnKBrZCGgv2ARjFNSZe7E7vGg5K4Q6Q8=\
+";
+
+//
+// Unexpired payment request (expires is set to 0x8000000000000000 > max. int64_t, allowed uint64)
+//
+const char* paymentrequest4_cert2_BASE64 =
+"\
+Egt4NTA5K3NoYTI1NhrQBArNBDCCAkkwggExoAMCAQICAQEwDQYJKoZIhvcNAQEL\
+BQAwITEfMB0GA1UEAwwWUGF5bWVudFJlcXVlc3QgVGVzdCBDQTAeFw0xNTAxMTEx\
+ODIxMDhaFw0yNTAxMDgxODIxMDhaMCExHzAdBgNVBAMMFlBheW1lbnRSZXF1ZXN0\
+IFRlc3QgQ0EwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMsZqzkzeBGo+i2N\
+mUak3Ciodr1V7S062VOy7N0OQYNDQHYkgDFAUET7cEb5VJaHPv5m3ppTBpU9xBcf\
+wbHHUt4VjA+mhRmYrl1khjvZM+X8kEqvWn20BtcM9R6r0yIYec8UERDDHBleL/P8\
+RkxEnVLjYTV9zigCXfMsgYb3EQShAgMBAAGjEDAOMAwGA1UdEwQFMAMBAf8wDQYJ\
+KoZIhvcNAQELBQADggEBABUJpl3QCqsoDSxAsQdV6zKT4VGV76AzoGj7etQsQY+r\
++S26VfWh/fMobEzuxFChr0USgLJ6FoK78hAtoZvt1lrye9yqFv/ig3WLWsJKWHHb\
+3RT6oR03CIwZXFSUasi08QDVLxafwsU5OMcPLucF3a1lRL1ccYrNgVCCx1+X7Bos\
+tIgDGRQQ4AyoHTcfVd2hEGeUv7k14mOxFsAp6851yosHq9Q2kwmdH+rHEJbjof87\
+yyKLagc4owyXBZYkQmkeHWCNqnuRmO5vUsfVb0UUrkD64o7Th/NjwooA7SCiUXl6\
+dfygT1b7ggpx7GC+sP2DsIM47IAZ55drjqX5u2f+Ba0iSwoEdGVzdBIgCICt4gQS\
+GXapFASsapRTBKxoykO9YhoackY1CqLyiKwYt+HZpQUggICAgICAgICAASoQVGVz\
+dGluZyB0ZXN0bmV0ISqAAXSQG8+GFA18VaKarlYrOz293rNMIub0swKGcQm8jAGX\
+HSLaRgHfUDeEPr4hydy4dtfu59KNwe2xsHOHu/SpO4L8SrA4Dm9A7SlNBVWdcLbw\
+d2hj739GDLz0b5KuJ2SG6VknMRQM976w/m2qlq0ccVGaaZ2zMIGfpzL3p6adwx/5\
+";
diff --git a/src/qt/test/paymentservertests.cpp b/src/qt/test/paymentservertests.cpp
index e6cdb58ef5..04935192c8 100644
--- a/src/qt/test/paymentservertests.cpp
+++ b/src/qt/test/paymentservertests.cpp
@@ -143,7 +143,38 @@ void PaymentServerTests::paymentServerTests()
QVERIFY(r.paymentRequest.IsInitialized());
QCOMPARE(PaymentServer::verifyNetwork(r.paymentRequest.getDetails()), false);
- // Just get some random data big enough to trigger BIP70 DoS protection
+ // Expired payment request (expires is set to 1 = 1970-01-01 00:00:01):
+ data = DecodeBase64(paymentrequest2_cert2_BASE64);
+ byteArray = QByteArray((const char*)&data[0], data.size());
+ r.paymentRequest.parse(byteArray);
+ // Ensure the request is initialized
+ QVERIFY(r.paymentRequest.IsInitialized());
+ // compares 1 < GetTime() == false (treated as expired payment request)
+ QCOMPARE(PaymentServer::verifyExpired(r.paymentRequest.getDetails()), true);
+
+ // Unexpired payment request (expires is set to 0x7FFFFFFFFFFFFFFF = max. int64_t):
+ // 9223372036854775807 (uint64), 9223372036854775807 (int64_t) and -1 (int32_t)
+ // -1 is 1969-12-31 23:59:59 (for a 32 bit time values)
+ data = DecodeBase64(paymentrequest3_cert2_BASE64);
+ byteArray = QByteArray((const char*)&data[0], data.size());
+ r.paymentRequest.parse(byteArray);
+ // Ensure the request is initialized
+ QVERIFY(r.paymentRequest.IsInitialized());
+ // compares 9223372036854775807 < GetTime() == false (treated as unexpired payment request)
+ QCOMPARE(PaymentServer::verifyExpired(r.paymentRequest.getDetails()), false);
+
+ // Unexpired payment request (expires is set to 0x8000000000000000 > max. int64_t, allowed uint64):
+ // 9223372036854775808 (uint64), -9223372036854775808 (int64_t) and 0 (int32_t)
+ // 0 is 1970-01-01 00:00:00 (for a 32 bit time values)
+ data = DecodeBase64(paymentrequest4_cert2_BASE64);
+ byteArray = QByteArray((const char*)&data[0], data.size());
+ r.paymentRequest.parse(byteArray);
+ // Ensure the request is initialized
+ QVERIFY(r.paymentRequest.IsInitialized());
+ // compares -9223372036854775808 < GetTime() == true (treated as expired payment request)
+ QCOMPARE(PaymentServer::verifyExpired(r.paymentRequest.getDetails()), true);
+
+ // Test BIP70 DoS protection:
unsigned char randData[BIP70_MAX_PAYMENTREQUEST_SIZE + 1];
GetRandBytes(randData, sizeof(randData));
// Write data to a temp file:
@@ -151,7 +182,6 @@ void PaymentServerTests::paymentServerTests()
tempFile.open();
tempFile.write((const char*)randData, sizeof(randData));
tempFile.close();
- // Trigger BIP70 DoS protection
QCOMPARE(PaymentServer::readPaymentRequestFromFile(tempFile.fileName(), r.paymentRequest), false);
delete server;
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 6006a7bd7b..57596b77bd 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -6,6 +6,7 @@
#include "addresstablemodel.h"
#include "guiconstants.h"
+#include "paymentserver.h"
#include "recentrequeststablemodel.h"
#include "transactiontablemodel.h"
@@ -294,11 +295,16 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran
LOCK2(cs_main, wallet->cs_wallet);
CWalletTx *newTx = transaction.getTransaction();
- // Store PaymentRequests in wtx.vOrderForm in wallet.
foreach(const SendCoinsRecipient &rcp, transaction.getRecipients())
{
if (rcp.paymentRequest.IsInitialized())
{
+ // Make sure any payment requests involved are still valid.
+ if (PaymentServer::verifyExpired(rcp.paymentRequest.getDetails())) {
+ return PaymentRequestExpired;
+ }
+
+ // Store PaymentRequests in wtx.vOrderForm in wallet.
std::string key("PaymentRequest");
std::string value;
rcp.paymentRequest.SerializeToString(&value);
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index d8df25f660..9916d11f93 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -40,7 +40,7 @@ public:
explicit SendCoinsRecipient(const QString &addr, const QString &label, const CAmount& amount, const QString &message):
address(addr), label(label), amount(amount), message(message), nVersion(SendCoinsRecipient::CURRENT_VERSION) {}
- // If from an insecure payment request, this is used for storing
+ // If from an unauthenticated payment request, this is used for storing
// the addresses, e.g. address-A<br />address-B<br />address-C.
// Info: As we don't need to process addresses in here when using
// payment requests, we can abuse it for displaying an address list.
@@ -111,7 +111,8 @@ public:
DuplicateAddress,
TransactionCreationFailed, // Error returned when wallet is still locked
TransactionCommitFailed,
- InsaneFee
+ InsaneFee,
+ PaymentRequestExpired
};
enum EncryptionStatus