From b19fc39317c4cc0510d93b86bb29b96b4404cfcc Mon Sep 17 00:00:00 2001 From: aleck099 Date: Sun, 26 Nov 2023 11:08:40 +0800 Subject: [PATCH] Fix heap-use-after-free of pending requests Translated versions of someday frequently crash on startup Pending requests got deleted by translation function, resulting use-after-free errors --- src/async_handler.cpp | 46 +++++++++++++++++++++++++------------------ src/async_handler.h | 2 +- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/async_handler.cpp b/src/async_handler.cpp index ffd5e1a14..3fe3cf805 100644 --- a/src/async_handler.cpp +++ b/src/async_handler.cpp @@ -43,7 +43,7 @@ //#define EP_DEBUG_SIMULATE_ASYNC namespace { - std::unordered_map async_requests; + std::unordered_map> async_requests; std::unordered_map file_mapping; int next_id = 0; #ifdef EMSCRIPTEN @@ -54,16 +54,18 @@ namespace { auto it = async_requests.find(path); if (it != async_requests.end()) { - return &(it->second); + return &*(it->second); } return nullptr; } FileRequestAsync* RegisterRequest(std::string path, std::string directory, std::string file) { - auto req = FileRequestAsync(path, std::move(directory), std::move(file)); - auto p = async_requests.emplace(std::make_pair(std::move(path), std::move(req))); - return &p.first->second; + auto p = async_requests.emplace( + std::move(path), + std::make_shared(path, std::move(directory), std::move(file)) + ); + return &*(p.first->second); } FileRequestBinding CreatePending() { @@ -75,7 +77,7 @@ namespace { struct async_download_context { std::string url, file, param; - FileRequestAsync* obj; + std::weak_ptr obj; size_t count; async_download_context( @@ -83,12 +85,15 @@ namespace { std::string f, std::string p, FileRequestAsync* o - ) : url{ std::move(u) }, file{ std::move(f) }, param{ std::move(p) }, obj{ o }, count{} {} + ) : url{ std::move(u) }, file{ std::move(f) }, param{ std::move(p) }, obj{ o->weak_from_this() }, count{} {} }; void download_success_retry(unsigned, void* userData, const char*) { auto ctx = static_cast(userData); - ctx->obj->DownloadDone(true); + auto sobj = ctx->obj.lock(); + // in case that object got deleted + if (sobj) + sobj->DownloadDone(true); delete ctx; } @@ -97,19 +102,22 @@ namespace { void download_failure_retry(unsigned, void* userData, int status) { auto ctx = static_cast(userData); ++ctx->count; - if (ctx->count >= ASYNC_MAX_RETRY_COUNT) { - Output::Warning("DL Failure: max retries exceeded: {}", ctx->obj->GetPath()); - ctx->obj->DownloadDone(false); - delete ctx; - return; - } - if (status >= 400) { - Output::Warning("DL Failure: file not available: {}", ctx->obj->GetPath()); - ctx->obj->DownloadDone(false); + bool flag1 = ctx->count >= ASYNC_MAX_RETRY_COUNT; + bool flag2 = status >= 400; + auto sobj = ctx->obj.lock(); + std::string_view download_path{ (sobj) ? sobj->GetPath() : "(deleted)" }; + if (flag1 || flag2) { + if (flag1) + Output::Warning("DL Failure: max retries exceeded: {}", download_path); + else if (flag2) + Output::Warning("DL Failure: file not available: {}", download_path); + + if (sobj) + sobj->DownloadDone(false); delete ctx; return; } - Output::Debug("DL Failure: {}. Retrying", ctx->obj->GetPath()); + Output::Debug("DL Failure: {}. Retrying", download_path); start_async_wget_with_retry(ctx); } @@ -240,7 +248,7 @@ FileRequestAsync* AsyncHandler::RequestFile(StringView file_name) { bool AsyncHandler::IsFilePending(bool important, bool graphic) { for (auto& ap: async_requests) { - FileRequestAsync& request = ap.second; + FileRequestAsync& request = *ap.second; #ifdef EP_DEBUG_SIMULATE_ASYNC request.UpdateProgress(); diff --git a/src/async_handler.h b/src/async_handler.h index acea9226b..4354b60fd 100644 --- a/src/async_handler.h +++ b/src/async_handler.h @@ -106,7 +106,7 @@ using FileRequestBindingWeak = std::weak_ptr; /** * Handles a single asynchronous file request. */ -class FileRequestAsync { +class FileRequestAsync : public std::enable_shared_from_this { public: enum AsyncState { State_WaitForStart,