From 971425d7b144ef7e5b80d0dab5678c78971d4b4b Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sat, 16 Jan 2021 11:48:23 +0100
Subject: ProtocolServer: Fix null dereference in HTTP/HTTPS job finish
 callback

The consolidation of the initialization code between HTTP and HTTPS
downloads was capturing the "job" local by reference, which was not safe
after we left the init() scope.

Fix this by getting the HTTP::Job from the Download object instead.
---
 Userland/Services/ProtocolServer/HttpCommon.h    | 10 +++++-----
 Userland/Services/ProtocolServer/HttpDownload.h  |  2 ++
 Userland/Services/ProtocolServer/HttpsDownload.h |  2 ++
 3 files changed, 9 insertions(+), 5 deletions(-)

(limited to 'Userland/Services/ProtocolServer')

diff --git a/Userland/Services/ProtocolServer/HttpCommon.h b/Userland/Services/ProtocolServer/HttpCommon.h
index 455af38d59..c42bd1471e 100644
--- a/Userland/Services/ProtocolServer/HttpCommon.h
+++ b/Userland/Services/ProtocolServer/HttpCommon.h
@@ -42,14 +42,14 @@ namespace ProtocolServer::Detail {
 template<typename TSelf, typename TJob>
 void init(TSelf* self, TJob job)
 {
-    job->on_headers_received = [&](auto& headers, auto response_code) {
+    job->on_headers_received = [self](auto& headers, auto response_code) {
         if (response_code.has_value())
             self->set_status_code(response_code.value());
         self->set_response_headers(headers);
     };
 
-    job->on_finish = [&](bool success) {
-        if (auto* response = job->response()) {
+    job->on_finish = [self](bool success) {
+        if (auto* response = self->job().response()) {
             self->set_status_code(response->code());
             self->set_response_headers(response->headers());
             self->set_downloaded_size(self->output_stream().size());
@@ -62,11 +62,11 @@ void init(TSelf* self, TJob job)
 
         self->did_finish(success);
     };
-    job->on_progress = [&](Optional<u32> total, u32 current) {
+    job->on_progress = [self](Optional<u32> total, u32 current) {
         self->did_progress(total, current);
     };
     if constexpr (requires { job->on_certificate_requested; }) {
-        job->on_certificate_requested = [&](auto&) {
+        job->on_certificate_requested = [self](auto&) {
             self->did_request_certificates();
         };
     }
diff --git a/Userland/Services/ProtocolServer/HttpDownload.h b/Userland/Services/ProtocolServer/HttpDownload.h
index 886d018942..6e7e5754ce 100644
--- a/Userland/Services/ProtocolServer/HttpDownload.h
+++ b/Userland/Services/ProtocolServer/HttpDownload.h
@@ -39,6 +39,8 @@ public:
     virtual ~HttpDownload() override;
     static NonnullOwnPtr<HttpDownload> create_with_job(Badge<HttpProtocol>&&, ClientConnection&, NonnullRefPtr<HTTP::HttpJob>, NonnullOwnPtr<OutputFileStream>&&);
 
+    HTTP::HttpJob& job() { return m_job; }
+
 private:
     explicit HttpDownload(ClientConnection&, NonnullRefPtr<HTTP::HttpJob>, NonnullOwnPtr<OutputFileStream>&&);
 
diff --git a/Userland/Services/ProtocolServer/HttpsDownload.h b/Userland/Services/ProtocolServer/HttpsDownload.h
index 988f3dc6c4..75fb131848 100644
--- a/Userland/Services/ProtocolServer/HttpsDownload.h
+++ b/Userland/Services/ProtocolServer/HttpsDownload.h
@@ -38,6 +38,8 @@ public:
     virtual ~HttpsDownload() override;
     static NonnullOwnPtr<HttpsDownload> create_with_job(Badge<HttpsProtocol>&&, ClientConnection&, NonnullRefPtr<HTTP::HttpsJob>, NonnullOwnPtr<OutputFileStream>&&);
 
+    HTTP::HttpsJob& job() { return m_job; }
+
 private:
     explicit HttpsDownload(ClientConnection&, NonnullRefPtr<HTTP::HttpsJob>, NonnullOwnPtr<OutputFileStream>&&);
 
-- 
cgit v1.2.3