neobytes-project/neobytes
neobytes-project/neobytes
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

[Qt] Payment request expiration bug fix (re-done)

Browse Source 
- this is based on #4122 (which can be closed)

Currently a payment request is only checked for expiration upon receipt.
It should be checked again immediately before sending coins to prevent
the user from paying to an expired invoice which would then require a
customer service interaction.

- add static verifyExpired() function to PaymentServer to be able to use
  the same validation code in GUI and unit-testing code
- extend unit tests to use that function and also add an unit test which
  overflows, because payment requests allow expires as uint64, whereas we
  use int64_t for verification of expired payment requests
This commit is contained in:
Philip Kaufmann 2015-01-08 14:42:04 +01:00
parent e0cd2f5523
commit 6715efb9ca
7 changed files with 137 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/qt/paymentserver.cpp
View File

@ -518,8 +518,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."),
@ -528,16 +526,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);
@ -756,3 +753,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;
}

2
src/qt/paymentserver.h
View File

@ -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

4
src/qt/sendcoinsdialog.cpp
View File

@ -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:

72
src/qt/test/paymentrequestdata.h
View File

@ -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\
";

34
src/qt/test/paymentservertests.cpp
View File

@ -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;

8
src/qt/walletmodel.cpp
View File

@ -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);

5
src/qt/walletmodel.h
View File

@ -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