From bdc83e8f450456c9f547f1c4eab43571bac631c2 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sat, 16 Nov 2013 01:54:29 +0100 Subject: [PATCH] [Qt] ensure payment request network matches client network - replaces checks in SendCoinsDialog::handlePaymentRequest() that belong to PaymentServer (normal URIs are special cased, as only an isValid check is done on BTC addresses) - prevents the client to handle payment requests that do not match the clients network and shows an error instead (mainly a problem with drag&drop payment requests onto the client window) - includes some small comment changes also --- src/qt/paymentserver.cpp | 81 ++++++++++++++++++++++++++++---------- src/qt/sendcoinsdialog.cpp | 22 +---------- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index ca6ae1799..4c4558568 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -178,6 +178,9 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store) // and the items in savedPaymentRequest will be handled // when uiReady() is called. // +// Warning: ipcSendCommandLine() is called early in init, +// so don't use "emit message()", but "QMessageBox::"! +// bool PaymentServer::ipcParseCommandLine(int argc, char* argv[]) { for (int i = 1; i < argc; i++) @@ -411,7 +414,15 @@ void PaymentServer::handleURIOrFile(const QString& s) { SendCoinsRecipient recipient; if (GUIUtil::parseBitcoinURI(s, &recipient)) - emit receivedPaymentRequest(recipient); + { + CBitcoinAddress address(recipient.address.toStdString()); + if (!address.IsValid()) { + emit message(tr("URI handling"), tr("Invalid payment address %1").arg(recipient.address), + CClientUIInterface::MSG_ERROR); + } + else + emit receivedPaymentRequest(recipient); + } else emit message(tr("URI handling"), tr("URI can not be parsed! This can be caused by an invalid Bitcoin address or malformed URI parameters."), @@ -425,12 +436,14 @@ void PaymentServer::handleURIOrFile(const QString& s) { PaymentRequestPlus request; SendCoinsRecipient recipient; - if (readPaymentRequest(s, request) && processPaymentRequest(request, recipient)) - emit receivedPaymentRequest(recipient); - else + if (!readPaymentRequest(s, request)) + { emit message(tr("Payment request file handling"), - tr("Payment request file can not be read or processed! This can be caused by an invalid payment request file."), + tr("Payment request file can not be read! This can be caused by an invalid payment request file."), CClientUIInterface::ICON_WARNING); + } + else if (processPaymentRequest(request, recipient)) + emit receivedPaymentRequest(recipient); return; } @@ -482,6 +495,35 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins if (!optionsModel) return false; + if (request.IsInitialized()) { + const payments::PaymentDetails& details = request.getDetails(); + + // Payment request network matches client network? + if ((details.network() == "main" && TestNet()) || + (details.network() == "test" && !TestNet())) + { + emit message(tr("Payment request rejected"), tr("Payment request network doesn't match client network."), + CClientUIInterface::MSG_ERROR); + + 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."), + CClientUIInterface::MSG_ERROR); + + return false; + } + } + else { + emit message(tr("Payment request error"), tr("Payment request is not initialized."), + CClientUIInterface::MSG_ERROR); + + return false; + } + recipient.paymentRequest = request; recipient.message = GUIUtil::HtmlEscape(request.getDetails().memo()); @@ -497,11 +539,11 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins // Append destination address addresses.append(QString::fromStdString(CBitcoinAddress(dest).ToString())); } - else if (!recipient.authenticatedMerchant.isEmpty()){ + else if (!recipient.authenticatedMerchant.isEmpty()) { // Insecure payments to custom bitcoin addresses are not supported // (there is no good way to tell the user where they are paying in a way // they'd have a chance of understanding). - emit message(tr("Payment request error"), + emit message(tr("Payment request rejected"), tr("Unverified payment requests to custom payment scripts are unsupported."), CClientUIInterface::MSG_ERROR); return false; @@ -510,11 +552,10 @@ bool PaymentServer::processPaymentRequest(PaymentRequestPlus& request, SendCoins // Extract and check amounts CTxOut txOut(sendingTo.second, sendingTo.first); if (txOut.IsDust(CTransaction::nMinRelayTxFee)) { - QString msg = tr("Requested payment amount of %1 is too small (considered dust).") - .arg(BitcoinUnits::formatWithUnit(optionsModel->getDisplayUnit(), sendingTo.second)); + emit message(tr("Payment request error"), tr("Requested payment amount of %1 is too small (considered dust).") + .arg(BitcoinUnits::formatWithUnit(optionsModel->getDisplayUnit(), sendingTo.second)), + CClientUIInterface::MSG_ERROR); - qDebug() << "PaymentServer::processPaymentRequest : " << msg; - emit message(tr("Payment request error"), msg, CClientUIInterface::MSG_ERROR); return false; } @@ -581,8 +622,8 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipien refund_to->set_script(&s[0], s.size()); } else { - // This should never happen, because sending coins should have just unlocked the wallet - // and refilled the keypool + // This should never happen, because sending coins should have + // just unlocked the wallet and refilled the keypool. qDebug() << "PaymentServer::fetchPaymentACK : Error getting refund key, refund_to not set"; } } @@ -594,7 +635,7 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, SendCoinsRecipient recipien netManager->post(netRequest, serData); } else { - // This should never happen, either: + // This should never happen, either. qDebug() << "PaymentServer::fetchPaymentACK : Error serializing payment message"; } } @@ -620,17 +661,15 @@ void PaymentServer::netRequestFinished(QNetworkReply* reply) { PaymentRequestPlus request; SendCoinsRecipient recipient; - if (request.parse(data) && processPaymentRequest(request, recipient)) + if (!request.parse(data)) { - emit receivedPaymentRequest(recipient); - } - else - { - qDebug() << "PaymentServer::netRequestFinished : Error processing payment request"; + qDebug() << "PaymentServer::netRequestFinished : Error parsing payment request"; emit message(tr("Payment request error"), - tr("Payment request can not be parsed or processed!"), + tr("Payment request can not be parsed!"), CClientUIInterface::MSG_ERROR); } + else if (processPaymentRequest(request, recipient)) + emit receivedPaymentRequest(recipient); return; } diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 33621e54b..23b8ef83e 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -377,26 +377,8 @@ void SendCoinsDialog::pasteEntry(const SendCoinsRecipient &rv) bool SendCoinsDialog::handlePaymentRequest(const SendCoinsRecipient &rv) { - QString strSendCoins = tr("Send Coins"); - if (rv.paymentRequest.IsInitialized()) { - // Expired payment request? - const payments::PaymentDetails& details = rv.paymentRequest.getDetails(); - if (details.has_expires() && (int64_t)details.expires() < GetTime()) - { - emit message(strSendCoins, tr("Payment request expired"), - CClientUIInterface::MSG_WARNING); - return false; - } - } - else { - CBitcoinAddress address(rv.address.toStdString()); - if (!address.IsValid()) { - emit message(strSendCoins, tr("Invalid payment address %1").arg(rv.address), - CClientUIInterface::MSG_WARNING); - return false; - } - } - + // Just paste the entry, all pre-checks + // are done in paymentserver.cpp. pasteEntry(rv); return true; }