Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Qt] add verifySize() function to PaymentServer #5665

Merged
merged 1 commit into from Sep 15, 2015
Merged

[Qt] add verifySize() function to PaymentServer #5665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 20 additions & 14 deletions src/qt/paymentserver.cpp
View file
Open in desktop
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/qt/paymentserver.h
View file
Open in desktop
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion src/qt/test/paymentservertests.cpp
View file
Open in desktop
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing this test?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be able to make readPaymentRequestFromFile private again, which I changed once and wasn't happy with it :). The function could also fail, if the file failed to open, which isn't what this test is for anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to be able to test a private methods you could use a friend class / function. E.g. inside the class do

friend class PaymentServerTester;

Then do the testing in a class PaymentServerTester.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laanwj It's not necessary here, but thanks for that C++ lesson :).

QCOMPARE(PaymentServer::verifySize(tempFile.size()), false);

// Payment request with amount overflow (amount is set to 21000001 BTC):
data = DecodeBase64(paymentrequest5_cert2_BASE64);
Expand Down