93 lines
3.2 KiB
Diff
93 lines
3.2 KiB
Diff
|
From f286ef4c5b3c71510d6ef15e8cc12cada84f3682 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Nicolas Fella <nicolas.fella@gmx.de>
|
||
|
|
Date: Sun, 27 Dec 2020 21:24:06 +0100
|
||
|
|
Subject: [PATCH] Fix use-after-free of QNetworkReply in BaseJob
|
||
|
|
|
||
|
|
Usually QNetworkAccessManager expects the user to delete the replies, but when the QNetworkAccessManager itself is deleted it deletes all pending replies (https://code.woboq.org/qt5/qtbase/src/network/access/qnetworkaccessmanager.cpp.html#529).
|
||
|
|
|
||
|
|
This can lead to use-after-free crashes when d->reply is accessed. By putting the reply into a QPointer the exiting if(d->reply) checks can work properly.
|
||
|
|
|
||
|
|
(cherry picked from commit 9d854e778d8d6ef8e03e1ea74fe958675b24fd45)
|
||
|
|
---
|
||
|
|
lib/jobs/basejob.cpp | 33 +++++++++++++++++++--------------
|
||
|
|
1 file changed, 19 insertions(+), 14 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp
|
||
|
|
index 3fa1cd94..2ac942f5 100644
|
||
|
|
--- a/lib/jobs/basejob.cpp
|
||
|
|
+++ b/lib/jobs/basejob.cpp
|
||
|
|
@@ -24,6 +24,7 @@
|
||
|
|
#include <QtCore/QTimer>
|
||
|
|
#include <QtCore/QStringBuilder>
|
||
|
|
#include <QtCore/QMetaEnum>
|
||
|
|
+#include <QtCore/QPointer>
|
||
|
|
#include <QtNetwork/QNetworkAccessManager>
|
||
|
|
#include <QtNetwork/QNetworkReply>
|
||
|
|
#include <QtNetwork/QNetworkRequest>
|
||
|
|
@@ -76,15 +77,6 @@ QDebug BaseJob::Status::dumpToLog(QDebug dbg) const
|
||
|
|
return dbg << ": " << message;
|
||
|
|
}
|
||
|
|
|
||
|
|
-struct NetworkReplyDeleter : public QScopedPointerDeleteLater {
|
||
|
|
- static inline void cleanup(QNetworkReply* reply)
|
||
|
|
- {
|
||
|
|
- if (reply && reply->isRunning())
|
||
|
|
- reply->abort();
|
||
|
|
- QScopedPointerDeleteLater::cleanup(reply);
|
||
|
|
- }
|
||
|
|
-};
|
||
|
|
-
|
||
|
|
template <typename... Ts>
|
||
|
|
constexpr auto make_array(Ts&&... items)
|
||
|
|
{
|
||
|
|
@@ -112,6 +104,16 @@ class BaseJob::Private {
|
||
|
|
retryTimer.setSingleShot(true);
|
||
|
|
}
|
||
|
|
|
||
|
|
+ ~Private()
|
||
|
|
+ {
|
||
|
|
+ if (reply) {
|
||
|
|
+ if (reply->isRunning()) {
|
||
|
|
+ reply->abort();
|
||
|
|
+ }
|
||
|
|
+ delete reply;
|
||
|
|
+ }
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
void sendRequest();
|
||
|
|
/*! \brief Parse the response byte array into JSON
|
||
|
|
*
|
||
|
|
@@ -140,7 +142,10 @@ class BaseJob::Private {
|
||
|
|
|
||
|
|
QByteArrayList expectedKeys;
|
||
|
|
|
||
|
|
- QScopedPointer<QNetworkReply, NetworkReplyDeleter> reply;
|
||
|
|
+ // When the QNetworkAccessManager is destroyed it destroys all pending replies.
|
||
|
|
+ // Using QPointer allows us to know when that happend.
|
||
|
|
+ QPointer<QNetworkReply> reply;
|
||
|
|
+
|
||
|
|
Status status = Unprepared;
|
||
|
|
QByteArray rawResponse;
|
||
|
|
/// Contains a null document in case of non-JSON body (for a successful
|
||
|
|
@@ -315,16 +320,16 @@ void BaseJob::Private::sendRequest()
|
||
|
|
|
||
|
|
switch (verb) {
|
||
|
|
case HttpVerb::Get:
|
||
|
|
- reply.reset(connection->nam()->get(req));
|
||
|
|
+ reply = connection->nam()->get(req);
|
||
|
|
break;
|
||
|
|
case HttpVerb::Post:
|
||
|
|
- reply.reset(connection->nam()->post(req, requestData.source()));
|
||
|
|
+ reply = connection->nam()->post(req, requestData.source());
|
||
|
|
break;
|
||
|
|
case HttpVerb::Put:
|
||
|
|
- reply.reset(connection->nam()->put(req, requestData.source()));
|
||
|
|
+ reply = connection->nam()->put(req, requestData.source());
|
||
|
|
break;
|
||
|
|
case HttpVerb::Delete:
|
||
|
|
- reply.reset(connection->nam()->sendCustomRequest(req, "DELETE", requestData.source()));
|
||
|
|
+ reply = connection->nam()->sendCustomRequest(req, "DELETE", requestData.source());
|
||
|
|
break;
|
||
|
|
}
|
||
|
|
}
|