From f99cf7ca3f9355e0562f965cd8eaccd4d2f28869 Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Mon, 28 Sep 2026 04:48:31 +0800 Subject: [PATCH] Sanitize Server-Supplied Download File Names The URL downloader used the file name from the Content-Disposition header as given, without the cleaning and unused-name search applied to the URL-derived name. Reduce the header name to a sanitized base name with the new sanitize_file_basename helper, which splits on both path separators and rejects names made only of dots and spaces. Run the result through the same unused-name search as the URL-derived name, now shared in find_unused_filename, and fall back to the URL-derived name when nothing usable remains. --- src/libslic3r/Utils.hpp | 8 ++++ src/slic3r/GUI/DownloaderFileGet.cpp | 61 +++++++++++++++++++--------- tests/libslic3r/test_utils.cpp | 24 +++++++++++ 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/libslic3r/Utils.hpp b/src/libslic3r/Utils.hpp index b21da72fc8..b1dcc81296 100644 --- a/src/libslic3r/Utils.hpp +++ b/src/libslic3r/Utils.hpp @@ -285,6 +285,14 @@ inline std::string sanitize_filename(const std::string &filename){ const std::regex special_chars("[/\\\\:*?\"<>|]"); return std::regex_replace(filename, special_chars, "_"); } +// Reduce an untrusted, possibly path-qualified name to a single sanitized file name. +// Returns an empty string when nothing usable remains. +inline std::string sanitize_file_basename(const std::string &name){ + const size_t sep = name.find_last_of("/\\"); + const std::string base = sanitize_filename(sep == std::string::npos ? name : name.substr(sep + 1)); + // Names made only of dots and spaces refer to the folder or its parent, or are stripped to nothing on Windows. + return base.find_first_not_of(". ") == std::string::npos ? std::string() : base; +} // File path / name / extension splitting utilities, working with UTF-8, // to be published to Perl. namespace PerlUtils { diff --git a/src/slic3r/GUI/DownloaderFileGet.cpp b/src/slic3r/GUI/DownloaderFileGet.cpp index 6ad07c43d2..e20f7dfa93 100644 --- a/src/slic3r/GUI/DownloaderFileGet.cpp +++ b/src/slic3r/GUI/DownloaderFileGet.cpp @@ -112,6 +112,7 @@ struct FileGet::priv priv(int ID, std::string&& url, const std::string& filename, wxEvtHandler* evt_handler, const boost::filesystem::path& dest_folder); void get_perform(); + std::string find_unused_filename(const std::string& filename) const; }; FileGet::priv::priv(int ID, std::string&& url, const std::string& filename, wxEvtHandler* evt_handler, const boost::filesystem::path& dest_folder) @@ -131,6 +132,24 @@ std::string extract_remote_filename(const std::string& str) { return ""; } } +// Returns a sanitized name that no file or other download in the destination folder uses yet, +// or an empty string when none is found. +std::string FileGet::priv::find_unused_filename(const std::string& filename) const +{ + const std::string extension = boost::filesystem::path(filename).extension().string(); + const std::string just_filename = filename.substr(0, filename.size() - extension.size()); + std::string final_filename = just_filename; + auto is_used = [this, &extension](const std::string& name) { + const boost::filesystem::path tmp_path = m_dest_folder / (name + extension + "." + std::to_string(get_current_pid()) + ".download"); + return boost::filesystem::exists(m_dest_folder / (name + extension)) || (tmp_path != m_tmp_path && boost::filesystem::exists(tmp_path)); + }; + for (size_t version = 1; is_used(final_filename); ++version) { + if (version > 999) + return {}; + final_filename = GUI::format("%1%(%2%)", just_filename, std::to_string(version)); + } + return sanitize_filename(final_filename + extension); +} void FileGet::priv::get_perform() { assert(m_evt_handler); @@ -144,25 +163,10 @@ void FileGet::priv::get_perform() std::string extension; if (m_written == 0) { - boost::filesystem::path dest_path = m_dest_folder / m_filename; - extension = dest_path.extension().string(); - std::string just_filename = m_filename.substr(0, m_filename.size() - extension.size()); - std::string final_filename = just_filename; - // Find unsed filename + extension = (m_dest_folder / m_filename).extension().string(); + std::string final_filename; try { - size_t version = 0; - while (boost::filesystem::exists(m_dest_folder / (final_filename + extension)) || boost::filesystem::exists(m_dest_folder / (final_filename + extension + "." + std::to_string(get_current_pid()) + ".download"))) - { - ++version; - if (version > 999) { - wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); - evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / (final_filename + extension)).string())); - evt->SetInt(m_id); - m_evt_handler->QueueEvent(evt); - return; - } - final_filename = GUI::format("%1%(%2%)", just_filename, std::to_string(version)); - } + final_filename = find_unused_filename(m_filename); } catch (const boost::filesystem::filesystem_error& e) { wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); @@ -171,8 +175,15 @@ void FileGet::priv::get_perform() m_evt_handler->QueueEvent(evt); return; } + if (final_filename.empty()) { + wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_ERROR); + evt->SetString(GUI::format_wxstr(L"Failed to find suitable filename. Last name: %1%." , (m_dest_folder / m_filename).string())); + evt->SetInt(m_id); + m_evt_handler->QueueEvent(evt); + return; + } - m_filename = sanitize_filename(final_filename + extension); + m_filename = final_filename; m_tmp_path = m_dest_folder / (m_filename + "." + std::to_string(get_current_pid()) + ".download"); @@ -221,7 +232,17 @@ void FileGet::priv::get_perform() if(dest_path.empty()) { std::string filename = extract_remote_filename(header); if (!filename.empty()) { - m_filename = filename; + // The name comes from the server: keep it inside the destination folder and never + // replace an existing file. Fall back to the URL-derived name if nothing usable remains. + filename = sanitize_file_basename(filename); + try { + if (!filename.empty()) + filename = find_unused_filename(filename); + } catch (const boost::filesystem::filesystem_error&) { + filename.clear(); + } + if (!filename.empty()) + m_filename = filename; dest_path = m_dest_folder / m_filename; wxCommandEvent* evt = new wxCommandEvent(EVT_DWNLDR_FILE_NAME_CHANGE); evt->SetString(boost::nowide::widen(m_filename)); diff --git a/tests/libslic3r/test_utils.cpp b/tests/libslic3r/test_utils.cpp index 7880b783f1..b71597c6d6 100644 --- a/tests/libslic3r/test_utils.cpp +++ b/tests/libslic3r/test_utils.cpp @@ -152,3 +152,27 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha REQUIRE(resolve_cli_input_path("").empty()); } } + +TEST_CASE("sanitize_file_basename keeps only a plain file name from an untrusted name", "[Utils]") { + const std::string unicode = "\xe6\xa8\xa1\xe5\x9e\x8b \xc3\xa9t\xc3\xa9.3mf"; // UTF-8 CJK and accented Latin + const auto [input, expected] = GENERATE_COPY(table({ + {"normal.3mf", "normal.3mf"}, + {"../../x.3mf", "x.3mf"}, + {"..\\..\\x.3mf", "x.3mf"}, + {"C:\\x.3mf", "x.3mf"}, + {"C:x.3mf", "C_x.3mf"}, + {"/etc/x", "x"}, + {"a/b\\c.gcode", "c.gcode"}, + {unicode, unicode}, + })); + CAPTURE(input); + const std::string name = sanitize_file_basename(input); + CHECK(name == expected); + CHECK(name.find_first_of("/\\:") == std::string::npos); +} + +TEST_CASE("sanitize_file_basename rejects names that do not name a file", "[Utils]") { + const std::string input = GENERATE(as{}, "", ".", "..", "../..", "dir/", "..\\", " ", ". .", "..."); + CAPTURE(input); + CHECK(sanitize_file_basename(input).empty()); +}