reapack

commit 5aed825fda6c31518200805cbc6a8f14a331a98d
parent d1569d4cb71e34c8cfec373e2aabbe687a819506
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Sat,  3 Feb 2024 10:14:20 -0500

don't interpret background task's item names as format strings

Fixes a crash when downloading files containing '%', and a pretty big security vulnerability.

The bug was introduced in ecdc99394a260ea6fe802af996d6e9fd38a63168.

Diffstat:
M
src/archive.cpp
 | 
4
++--
M
src/download.cpp
 | 
2
+-
M
src/progress.cpp
 | 
16
++++++++--------
M
src/progress.hpp
 | 
5
++---
M
src/thread.hpp
 | 
11
++++++++---

5 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/archive.cpp b/src/archive.cpp
@@ -221,7 +221,7 @@ int ArchiveReader::extractFile(const Path &path, std::ostream &stream) noexcept
 FileExtractor::FileExtractor(const Path &target, const ArchiveReaderPtr &reader)
   : m_path(target), m_reader(reader)
 {
-  setSummary("Extracting %s: " + target.join());
+  setSummary({ "Extracting", target.join() });
 }
 
 bool FileExtractor::run()
@@ -303,7 +303,7 @@ int ArchiveWriter::addFile(const Path &path, std::istream &stream) noexcept
 FileCompressor::FileCompressor(const Path &target, const ArchiveWriterPtr &writer)
   : m_path(target), m_writer(writer)
 {
-  setSummary("Compressing %s: " + target.join());
+  setSummary({ "Compressing", target.join() });
 }
 
 bool FileCompressor::run()
diff --git a/src/download.cpp b/src/download.cpp
@@ -111,7 +111,7 @@ Download::Download(const std::string &url, const NetworkOpts &opts, const int fl
 
 void Download::setName(const std::string &name)
 {
-  setSummary("Downloading %s: " + name);
+  setSummary({ "Downloading", name });
 }
 
 bool Download::run()
diff --git a/src/progress.cpp b/src/progress.cpp
@@ -17,7 +17,6 @@
 
 #include "progress.hpp"
 
-#include "thread.hpp"
 #include "resource.hpp"
 #include "win32.hpp"
 
@@ -25,7 +24,7 @@
 
 Progress::Progress(ThreadPool *pool)
   : Dialog(IDD_PROGRESS_DIALOG),
-    m_pool(pool), m_label(nullptr), m_progress(nullptr),
+    m_pool(pool), m_current(), m_label(nullptr), m_progress(nullptr),
     m_done(0), m_total(0)
 {
   m_pool->onPush >> std::bind(&Progress::addTask, this, std::placeholders::_1);
@@ -68,7 +67,8 @@ void Progress::onTimer(const int id)
 void Progress::addTask(ThreadTask *task)
 {
   m_total++;
-  updateProgress();
+  if(m_current.step)
+    updateProgress();
 
   if(!isVisible())
     startTimer(100);
@@ -86,11 +86,11 @@ void Progress::addTask(ThreadTask *task)
 
 void Progress::updateProgress()
 {
-  Win32::setWindowText(m_label, String::format(m_current.c_str(),
-    String::format("%s of %s",
-      String::number(std::min(m_done + 1, m_total)).c_str(),
-      String::number(m_total).c_str()
-    ).c_str()
+  Win32::setWindowText(m_label, String::format("%s %s of %s: %s",
+    m_current.step,
+    String::number(std::min(m_done + 1, m_total)).c_str(),
+    String::number(m_total).c_str(),
+    m_current.item.c_str()
   ).c_str());
 
   const double pos = static_cast<double>(
diff --git a/src/progress.hpp b/src/progress.hpp
@@ -20,8 +20,7 @@
 
 #include "dialog.hpp"
 
-class ThreadPool;
-class ThreadTask;
+#include "thread.hpp"
 
 class Progress : public Dialog {
 public:
@@ -37,7 +36,7 @@ private:
   void updateProgress();
 
   ThreadPool *m_pool;
-  std::string m_current;
+  ThreadSummary m_current;
 
   HWND m_label;
   HWND m_progress;
diff --git a/src/thread.hpp b/src/thread.hpp
@@ -28,6 +28,11 @@
 #include <thread>
 #include <unordered_set>
 
+struct ThreadSummary {
+  const char *step;
+  std::string item;
+};
+
 class ThreadTask {
 public:
   enum State {
@@ -45,7 +50,7 @@ public:
   virtual bool concurrent() const = 0;
 
   void exec();  // runs in the current thread
-  const std::string &summary() const { return m_summary; }
+  const ThreadSummary &summary() const { return m_summary; }
   State state() const { return m_state; }
   void setError(const ErrorInfo &err) { m_error = err; }
   const ErrorInfo &error() { return m_error; }
@@ -59,10 +64,10 @@ public:
 protected:
   virtual bool run() = 0;
 
-  void setSummary(const std::string &s) { m_summary = s; }
+  void setSummary(const ThreadSummary &s) { m_summary = s; }
 
 private:
-  std::string m_summary;
+  ThreadSummary m_summary;
   State m_state;
   ErrorInfo m_error;
   std::atomic_bool m_abort;