Validate Plugin Symlink Targets Before Replacing Existing Files

A symlink entry's target is now read and checked before anything already
at its destination is removed or renamed aside, so an archive rejected
for its link target leaves the installed plugin files in place.

resources/web/model/model.js

@@ -214,9 +214,9 @@ function ShowModelInfo( pModel )
 	
 	SendWXDebugInfo("Model Name:  "+sModelName);
 	
-	$('#ModelName').text(sModelName);
+	$('#ModelName').html(sModelName);
 	$('#ModelName').attr('title',sModelName);
-    $('#ModelAuthorName').text(sModelAuthor);
+    $('#ModelAuthorName').html(sModelAuthor);
 	
 	switch(UploadType)
 	{
@@ -268,7 +268,7 @@ function ShowModelInfo( pModel )
 			break;
 	}
 	
-	$('#Model_Desc').empty().append( SanitizeDescHtml( html_decode(sModelDesc) ) );
+	$('#Model_Desc').html( html_decode(sModelDesc) );
 			
 	let ModelPreviewList=pModel.preview_img;				
     let TotalPreview=ModelPreviewList.length;
@@ -281,15 +281,16 @@ function ShowModelInfo( pModel )
 	
     if(TotalPreview>0)
 	{
-		$('#ModelPreviewList').empty();
+		let htmlPreview='';
 		for(let pn=0;pn<TotalPreview;pn++)			
 		{	
 			//let FTmpPath=decodeURIComponent(ModelPreviewList[pn]);
 			let FTmpPath=ModelPreviewList[pn]['filepath'];
 			
-			$('#ModelPreviewList').append( $('<div class="swiper-slide"></div>').append( $('<img class="Model_PrevImg" />').attr('src',FTmpPath) ) );
+			htmlPreview+='<div class="swiper-slide"><img class="Model_PrevImg" src="'+FTmpPath+'" /></div>';
 		}
 			
+	    $('#ModelPreviewList').html(htmlPreview);
 		$('#Model_Preview_Image').viewer({
 			title: false,
 		    fullsreen: false,
@@ -409,8 +410,7 @@ function ConstructFileHtml( ID, pItem )
 {
 	let fTotal=pItem.length;
 	
-	let pBoard=$('#'+ID+'  .FileListBoard');
-	pBoard.empty();
+	let strHtml='';
 	for( let f=0;f<fTotal;f++ )
 	{
 		let pOne=pItem[f];
@@ -443,139 +443,39 @@ function ConstructFileHtml( ID, pItem )
 			ImgPath='img/default.png';			
 		}		
 			
-		//Add html. File names come from the 3MF, so build the nodes rather than concatenating markup.
-		let pIconImg=$('<img />').attr('src',ImgPath);
-		let pMenu=$('<div class="FileMenu"><img src="img/s.svg" /></div>');
+		//Add html
 		if( strClass!='ImageIcon' )
 		{
-			pMenu.on('click', function(){ OnClickOpenFile(tPath); });
+		strHtml+='<div class="FileItem">'+
+				 '	<div class="'+strClass+'"><img src="'+ImgPath+'" /></div>'+
+				 '	<div class="FileText">'+
+			     '		<div class="FileName">'+tName+'</div>'+
+				 '	</div>'+
+				 '	<div class="FileMenu" onClick="OnClickOpenFile(\''+tPath+'\')"><img src="img/s.svg" /></div>'+
+				 '</div>';
 		}
 		else
 		{
 			ImgID++;
 			let TmpImgID="AF"+ImgID;
 			
-			pIconImg.attr('id',TmpImgID);
-			pMenu.on('click', function(){ OnClickOpenImage(TmpImgID); });
+		strHtml+='<div class="FileItem">'+
+				 '	<div class="'+strClass+'"><img id="'+TmpImgID+'" src="'+ImgPath+'" /></div>'+
+				 '	<div class="FileText">'+
+			     '		<div class="FileName">'+tName+'</div>'+
+				 '	</div>'+
+				 '	<div class="FileMenu" onClick="OnClickOpenImage(\''+TmpImgID+'\')"><img src="img/s.svg" /></div>'+
+				 '</div>';			
 		}
-		
-		let pFileItem=$('<div class="FileItem"></div>');
-		pFileItem.append( $('<div></div>').addClass(strClass).append(pIconImg) );
-		pFileItem.append( $('<div class="FileText"></div>').append( $('<div class="FileName"></div>').text(tName).attr('title',tName) ) );
-		pFileItem.append( pMenu );
-		pBoard.append( pFileItem );
 	}
 	
+	$('#'+ID+'  .FileListBoard').html(strHtml);
+	
     if( fTotal>0 )
 		$('#'+ID).show();
 }
 
 
-// Descriptions are untrusted 3MF metadata that may carry rich-text HTML (e.g. from MakerWorld).
-// Rebuild them from an inert parse, keeping only plain formatting tags and http(s) links and images.
-var DescAllowedTags=['P','BR','B','STRONG','I','EM','U','S','STRIKE','DEL','INS','SUB','SUP','SMALL','MARK',
-	'Q','ABBR','KBD','WBR','H1','H2','H3','H4','H5','H6','UL','OL','LI','DL','DT','DD','BLOCKQUOTE','PRE','CODE',
-	'HR','SPAN','DIV','FIGURE','FIGCAPTION','TABLE','CAPTION','THEAD','TBODY','TFOOT','TR','TH','TD','A','IMG'];
-// Plain attributes kept per tag; the numeric ones must be plain non-negative integers.
-var DescAllowedAttrs={'IMG':['alt','title','width','height'],'TD':['colspan','rowspan'],'TH':['colspan','rowspan'],'OL':['start']};
-var DescNumericAttrs=['width','height','colspan','rowspan','start'];
-// Dropped together with their content; any other unknown tag is unwrapped to its children.
-var DescDroppedTags=['SCRIPT','STYLE','TEMPLATE','NOSCRIPT','TEXTAREA','TITLE','IFRAME','FRAME','OBJECT','EMBED','SVG','MATH'];
-
-function IsHttpUrl( strUrl )
-{
-	// The scheme must be written as is, so nothing the URL parser would strip can precede or split it.
-	if( typeof strUrl!='string' || !/^https?:/i.test(strUrl) )
-		return false;
-	try
-	{
-		let sProtocol=new URL(strUrl).protocol;
-		return sProtocol=='http:' || sProtocol=='https:';
-	}
-	catch(e)
-	{
-		return false;
-	}
-}
-
-// Images load as soon as the page opens, so only https sources are kept: no plain-http requests to the local network.
-function IsHttpsUrl( strUrl )
-{
-	return IsHttpUrl(strUrl) && new URL(strUrl).protocol=='https:';
-}
-
-// Embedded YouTube players become a plain link to the video.
-function GetYouTubeEmbedUrl( pNode )
-{
-	let sSrc=pNode.getAttribute('src');
-	if( !IsHttpUrl(sSrc) )
-		return null;
-	let pUrl=new URL(sSrc);
-	return ( pUrl.origin=='https://www.youtube.com' && pUrl.pathname.indexOf('/embed/')==0 ) ? pUrl.href : null;
-}
-
-function CopyDescNodes( pSrc, pDst )
-{
-	for( let pNode=pSrc.firstChild;pNode!=null;pNode=pNode.nextSibling )
-	{
-		if( pNode.nodeType==Node.TEXT_NODE )
-		{
-			pDst.appendChild( document.createTextNode(pNode.nodeValue) );
-			continue;
-		}
-		if( pNode.nodeType!=Node.ELEMENT_NODE )
-			continue;
-		
-		let sTag=pNode.nodeName.toUpperCase();
-		if( sTag=='IFRAME' )
-		{
-			let sVideoUrl=GetYouTubeEmbedUrl(pNode);
-			if( sVideoUrl!=null )
-			{
-				let pLink=document.createElement('A');
-				pLink.setAttribute('href',sVideoUrl);
-				pLink.textContent=sVideoUrl;
-				pDst.appendChild(pLink);
-			}
-			continue;
-		}
-		if( $.inArray(sTag,DescDroppedTags)>=0 )
-			continue;
-		if( $.inArray(sTag,DescAllowedTags)<0 )
-		{
-			CopyDescNodes(pNode,pDst);
-			continue;
-		}
-		
-		let pElem=document.createElement(sTag);
-		if( sTag=='A' && IsHttpUrl(pNode.getAttribute('href')) )
-			pElem.setAttribute('href',pNode.getAttribute('href'));
-		else if( sTag=='IMG' )
-		{
-			if( !IsHttpsUrl(pNode.getAttribute('src')) )
-				continue;
-			pElem.setAttribute('src',pNode.getAttribute('src'));
-		}
-		$.each( DescAllowedAttrs[sTag]||[], function(i,sAttr){
-			let sValue=pNode.getAttribute(sAttr);
-			if( sValue!=null && ( $.inArray(sAttr,DescNumericAttrs)<0 || /^\d+$/.test(sValue) ) )
-				pElem.setAttribute(sAttr,sValue);
-		});
-		CopyDescNodes(pNode,pElem);
-		pDst.appendChild(pElem);
-	}
-}
-
-function SanitizeDescHtml( strHtml )
-{
-	let pFragment=document.createDocumentFragment();
-	// A DOMParser document is inert: it runs no scripts and loads no resources.
-	let pDoc=new DOMParser().parseFromString(strHtml,'text/html');
-	if( pDoc && pDoc.body )
-		CopyDescNodes(pDoc.body,pFragment);
-	return pFragment;
-}
-
 function ShowProfilelInfo( pProfile )
 {
 	//==========Profile Info==========
@@ -583,10 +483,10 @@ function ShowProfilelInfo( pProfile )
 	let sProfileAuthor=decodeURIComponent(pProfile.author);
 	let sProfileDesc=decodeURIComponent(pProfile.description);
 	
-	$('#ProfileName').text(sProfileName);
-    $('#ProfileAuthor').text(sProfileAuthor);
+	$('#ProfileName').html(sProfileName);
+    $('#ProfileAuthor').html(sProfileAuthor);
 		
-	$('#Profile_Desc').empty().append( SanitizeDescHtml( html_decode(sProfileDesc) ) );
+	$('#Profile_Desc').html( html_decode(sProfileDesc) );
 			
 	let ProfilePreviewList=pProfile.preview_img;				
     let TotalPreview=ProfilePreviewList.length;
@@ -599,14 +499,15 @@ function ShowProfilelInfo( pProfile )
 	
     if(TotalPreview>0)
 	{
-		$('#ProfilePreviewList').empty();
+		let htmlPreview='';
 		for(let pn=0;pn<TotalPreview;pn++)			
 		{	
 			let FTmpPath=ProfilePreviewList[pn]['filepath'];
 			
-			$('#ProfilePreviewList').append( $('<div class="swiper-slide"></div>').append( $('<img class="Model_PrevImg" />').attr('src',FTmpPath) ) );
+			htmlPreview+='<div class="swiper-slide"><img class="Model_PrevImg" src="'+FTmpPath+'" /></div>';
 		}
 			
+		$('#ProfilePreviewList').html(htmlPreview);
 		$('#Profile_Preview_Image').viewer({
 			title: false,
 		    fullsreen: false,

src/libslic3r/Utils.hpp

@@ -260,11 +260,9 @@ extern bool is_json_file(const std::string& path);
 // Both '/' and '\\' are treated as separators on every platform, so an archive rejected on one OS
 // is rejected on all of them.
 extern bool is_path_within_root(const std::string &rel_path, const boost::filesystem::path &root);
-// True if path names an entry strictly inside root: it must be spelled with root as its prefix,
-// and must still resolve inside root once symlinks are followed.
-extern bool is_absolute_path_within_root(const boost::filesystem::path &path, const boost::filesystem::path &root);
-// True if opening a file with this name through the desktop would run it as a program or script.
-extern bool is_executable_file_name(const std::string &file_name);
+// True if a symlink stored at link_rel_path (relative to root) with this target stays inside root: the target
+// must be relative, and joined to the link's directory it must pass is_path_within_root.
+extern bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, const boost::filesystem::path &root);
 
 // Orca: custom protocal support utils
 inline bool is_orca_open(const std::string& url) { return boost::starts_with(url, "orcaslicer://open"); }

src/libslic3r/miniz_extension.cpp

@@ -4,6 +4,9 @@
 #include "miniz_extension.hpp"
 #include "Utils.hpp"
 
+#include <boost/filesystem.hpp>
+#include <boost/log/trivial.hpp>
+
 #if defined(_MSC_VER) || defined(__MINGW64__)
 #include "boost/nowide/cstdio.hpp"
 #endif
@@ -115,6 +118,66 @@ std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_
     return decode_zip_unicode_path_extra_field(extra.substr(0, extra_size > 0 ? extra_size - 1 : 0), stat.m_filename);
 }
 
+bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir)
+{
+    mz_zip_archive archive;
+    mz_zip_zero_struct(&archive);
+
+    if (!open_zip_reader(&archive, zip_path_utf8)) {
+        BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for " << zip_path_utf8;
+        return false;
+    }
+
+    const mz_uint num_entries = mz_zip_reader_get_num_files(&archive);
+    mz_zip_archive_file_stat stat;
+
+    // Validate every entry first so an archive with a single escaping entry leaves no partial output behind.
+    const boost::filesystem::path root(dest_dir);
+    for (mz_uint i = 0; i < num_entries; ++i) {
+        if (mz_zip_reader_file_stat(&archive, i, &stat) && !is_path_within_root(stat.m_filename, root)) {
+            BOOST_LOG_TRIVIAL(error) << "Unzip: rejecting " << zip_path_utf8 << ", entry " << stat.m_filename << " resolves outside " << dest_dir;
+            close_zip_reader(&archive);
+            return false;
+        }
+    }
+
+    for (mz_uint i = 0; i < num_entries; ++i) {
+        if (!mz_zip_reader_file_stat(&archive, i, &stat)) {
+            BOOST_LOG_TRIVIAL(warning) << "Unzip: read file stat failed";
+            continue;
+        }
+        const std::string dest_file = dest_dir + "/" + stat.m_filename;
+        try {
+            if (stat.m_is_directory) {
+                const boost::filesystem::path dest_path(dest_file);
+                if (!boost::filesystem::exists(dest_path))
+                    boost::filesystem::create_directories(dest_path);
+                continue;
+            }
+            if (stat.m_uncomp_size == 0) {
+                BOOST_LOG_TRIVIAL(warning) << "Unzip: invalid size for file " << stat.m_filename;
+                continue;
+            }
+            // Replace a symlink at the destination rather than writing through it.
+            const boost::filesystem::path dest_path(dest_file);
+            if (boost::filesystem::is_symlink(boost::filesystem::symlink_status(dest_path)))
+                boost::filesystem::remove(dest_path);
+            if (!mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0)) {
+                BOOST_LOG_TRIVIAL(error) << "Unzip: extract file " << stat.m_filename << " to dest " << dest_file << " failed";
+                close_zip_reader(&archive);
+                return false;
+            }
+            BOOST_LOG_TRIVIAL(info) << "Unzip: successfully extract file " << stat.m_file_index << " to " << dest_file;
+        } catch (const std::exception &e) {
+            close_zip_reader(&archive);
+            BOOST_LOG_TRIVIAL(error) << "Unzip: archive read exception: " << e.what();
+            return false;
+        }
+    }
+    close_zip_reader(&archive);
+    return true;
+}
+
 MZ_Archive::MZ_Archive()
 {
     mz_zip_zero_struct(&arch);

src/libslic3r/miniz_extension.hpp

@@ -11,6 +11,8 @@ bool open_zip_writer(mz_zip_archive *zip, const std::string &fname_utf8);
 bool close_zip_reader(mz_zip_archive *zip);
 bool close_zip_writer(mz_zip_archive *zip);
 std::string decode_archive_entry_path(mz_zip_archive *zip, const mz_zip_archive_file_stat &stat);
+// Extracts every entry of the archive under dest_dir. Nothing is written if any entry would resolve outside dest_dir.
+bool extract_archive_confined(const std::string &zip_path_utf8, const std::string &dest_dir);
 
 class MZ_Archive {
 public:

src/libslic3r/utils.cpp

@@ -70,7 +70,6 @@
 #include <boost/shared_ptr.hpp>
 
 #include <boost/algorithm/string/predicate.hpp>
-#include <boost/algorithm/string/case_conv.hpp>
 #include <boost/filesystem.hpp>
 #include <boost/filesystem/path.hpp>
 #include <boost/nowide/fstream.hpp>
@@ -1104,7 +1103,10 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
     }
     // Resolve against the canonical root so a symlink inside it cannot lead back out.
     try {
-        const std::string root_str = boost::filesystem::weakly_canonical(root).string();
+        std::string root_str = boost::filesystem::weakly_canonical(root).string();
+        // A trailing separator on root would otherwise fail the prefix match below for every path.
+        while (!root_str.empty() && (root_str.back() == '/' || root_str.back() == boost::filesystem::path::preferred_separator))
+            root_str.pop_back();
         const std::string full_str = boost::filesystem::weakly_canonical(root / rel_path).string();
         return full_str.compare(0, root_str.size(), root_str) == 0 &&
                (full_str.size() == root_str.size() || full_str[root_str.size()] == boost::filesystem::path::preferred_separator);
@@ -1113,32 +1115,13 @@ bool is_path_within_root(const std::string &rel_path, const boost::filesystem::p
     }
 }
 
-bool is_absolute_path_within_root(const boost::filesystem::path &path, const boost::filesystem::path &root)
+bool is_symlink_target_within_root(const std::string &link_rel_path, const std::string &target, const boost::filesystem::path &root)
 {
-    const boost::filesystem::path rel = path.lexically_relative(root);
-    return !rel.empty() && rel != "." && is_path_within_root(rel.string(), root);
-}
-
-bool is_executable_file_name(const std::string &file_name)
-{
-    static const std::vector<std::string> executable_extensions = {
-        // Windows
-        "exe", "com", "bat", "cmd", "scr", "pif", "msi", "msp", "msc", "cpl", "lnk", "url", "hta", "js", "jse", "vbs",
-        "vbe", "wsf", "wsh", "ps1", "psm1", "reg", "jar", "appref-ms", "scf", "inf", "py", "pyw",
-        // macOS
-        "app", "command", "pkg", "terminal", "workflow",
-        // Linux
-        "sh", "bash", "run", "desktop", "appimage"};
-    // Windows ignores trailing dots and spaces, so "setup.exe." still runs as an .exe.
-    const size_t end = file_name.find_last_not_of(". ");
-    if (end == std::string::npos)
+    if (target.empty() || target.front() == '/' || target.front() == '\\' || (target.size() > 1 && target[1] == ':'))
         return false;
-    const std::string name = file_name.substr(0, end + 1);
-    const size_t      dot  = name.find_last_of('.');
-    if (dot == std::string::npos || name.find_first_of("/\\", dot) != std::string::npos)
-        return false;
-    const std::string extension = boost::algorithm::to_lower_copy(name.substr(dot + 1));
-    return std::find(executable_extensions.begin(), executable_extensions.end(), extension) != executable_extensions.end();
+    // A relative target without ".." only descends from the link's directory, so no chain of such links can leave root.
+    const size_t sep = link_rel_path.find_last_of("/\\");
+    return is_path_within_root((sep == std::string::npos ? std::string() : link_rel_path.substr(0, sep + 1)) + target, root);
 }
 
 bool is_img_file(const std::string &path)

src/slic3r/GUI/GUI_App.cpp

@@ -1512,11 +1512,33 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP
                     size_t n = mz_zip_reader_get_extra(&archive, stat.m_file_index, extra.data(), extra.size());
                     dest_file = decode(extra.substr(0, n), stat.m_filename);
                 }
+                if (!is_path_within_root(dest_file, plugin_folder)) {
+                    BOOST_LOG_TRIVIAL(error) << "[install_plugin] entry " << dest_file << " resolves outside " << plugin_folder.string();
+                    close_zip_reader(&archive);
+                    if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); }
+                    return InstallStatusUnzipFailed;
+                }
                 auto dest_path = plugin_folder / dest_file;
-                boost::filesystem::create_directories(dest_path.parent_path());
                 std::string dest_zip_file = encode_path(dest_path.string().c_str());
+#ifndef WIN32
+                // Validate a symlink's target before anything at the destination is replaced.
+                const bool is_link = S_ISLNK(stat.m_external_attr >> 16);
+                std::string link;
+                if (is_link) {
+                    link.assign(stat.m_uncomp_size, 0);
+                    if (!mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0) ||
+                        !is_symlink_target_within_root(dest_file, link, plugin_folder)) {
+                        BOOST_LOG_TRIVIAL(error) << "[install_plugin] link " << dest_file << " -> " << link << " is unreadable or resolves outside " << plugin_folder.string();
+                        close_zip_reader(&archive);
+                        if (pro_fn) { pro_fn(InstallStatusUnzipFailed, 0, cancel); }
+                        return InstallStatusUnzipFailed;
+                    }
+                }
+#endif
                 try {
-                    if (fs::exists(dest_path)) {
+                    boost::filesystem::create_directories(dest_path.parent_path());
+                    // symlink_status so that an existing symlink, dangling or not, is replaced rather than written through.
+                    if (fs::exists(fs::symlink_status(dest_path))) {
                         boost::system::error_code ec;
                         fs::remove(dest_path, ec);
                         if (ec) {
@@ -1544,9 +1566,8 @@ int GUI_App::install_plugin(std::string name, std::string package_name, InstallP
                     }
                     mz_bool res = 0;
 #ifndef WIN32
-                    if (S_ISLNK(stat.m_external_attr >> 16)) {
-                        std::string link(stat.m_uncomp_size + 1, 0);
-                        res = mz_zip_reader_extract_to_mem(&archive, stat.m_file_index, link.data(), stat.m_uncomp_size, 0);
+                    if (is_link) {
+                        res = 1;
                         try {
                             boost::filesystem::create_symlink(link, dest_path);
                         } catch (const std::exception &e) {

src/slic3r/GUI/Project.cpp

@@ -27,7 +27,6 @@
 #include "GUI_App.hpp"
 #include "GUI_ObjectList.hpp"
 #include "MainFrame.hpp"
-#include "MsgDialog.hpp"
 #include <slic3r/GUI/Widgets/WebView.hpp>
 
 namespace Slic3r { namespace GUI {
@@ -294,24 +293,9 @@ void ProjectPanel::OnScriptMessage(wxWebViewEvent& evt)
             if (!accessory_path.empty()) {
                 std::string decode_path = wxGetApp().url_decode(accessory_path.ToStdString());
                 fs::path path(decode_path);
-                // Only open the project's own extracted auxiliary files, with the root built as in Reload().
-                fs::path aux_root(encode_path(wxGetApp().plater()->model().get_auxiliary_file_temp_path().c_str()));
 
-                if (is_absolute_path_within_root(path, aux_root) && fs::is_regular_file(path)) {
-                    // Attachments come with the project, so ask before running one that is a program or script.
-                    bool open = true;
-                    if (is_executable_file_name(path.filename().string())) {
-                        MessageDialog dlg(this,
-                                          wxString::Format(_L("\"%s\" is a program or script. Opening it will run it on this computer.\n\n"
-                                                              "Only open attachments from projects you trust. Open it anyway?"),
-                                                           from_path(path.filename())),
-                                          _L("Open attachment"), wxICON_WARNING | wxYES_NO);
-                        open = dlg.ShowModal() == wxID_YES;
-                    }
-                    if (open)
-                        wxLaunchDefaultApplication(path.wstring(), 0);
-                } else {
-                    BOOST_LOG_TRIVIAL(warning) << "open_3mf_accessory: ignoring path outside the project auxiliary directory: " << decode_path;
+                if (fs::exists(path)) {
+                    wxLaunchDefaultApplication(path.wstring(), 0);
                 }
             }
         }

src/slic3r/Utils/PresetUpdater.cpp

@@ -339,62 +339,8 @@ bool PresetUpdater::priv::get_file(const std::string &url, const fs::path &targe
 //BBS: refine preset update logic
 bool PresetUpdater::priv::extract_file(const fs::path &source_path, const fs::path &dest_path)
 {
-    bool res = true;
-    std::string file_path = source_path.string();
-    std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string();
-    mz_zip_archive archive;
-    mz_zip_zero_struct(&archive);
-
-    if (!open_zip_reader(&archive, file_path))
-    {
-        BOOST_LOG_TRIVIAL(error) << "Unable to open zip reader for "<<file_path;
-        return false;
-    }
-
-    mz_uint num_entries = mz_zip_reader_get_num_files(&archive);
-
-    mz_zip_archive_file_stat stat;
-    // we first loop the entries to read from the archive the .amf file only, in order to extract the version from it
-    for (mz_uint i = 0; i < num_entries; ++i)
-    {
-        if (mz_zip_reader_file_stat(&archive, i, &stat))
-        {
-            std::string dest_file = parent_path+"/"+stat.m_filename;
-            if (stat.m_is_directory) {
-                fs::path dest_path(dest_file);
-                if (!fs::exists(dest_path))
-                    fs::create_directories(dest_path);
-				continue;
-            }
-            else if (stat.m_uncomp_size == 0) {
-                BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: invalid size for file "<<stat.m_filename;
-                continue;
-            }
-            try
-            {
-                res = mz_zip_reader_extract_to_file(&archive, stat.m_file_index, dest_file.c_str(), 0);
-                if (!res) {
-                    BOOST_LOG_TRIVIAL(error) << "[Orca Updater]extract file "<<stat.m_filename<<" to dest "<<dest_file<<" failed";
-                    close_zip_reader(&archive);
-                    return res;
-                }
-                BOOST_LOG_TRIVIAL(info) << "[Orca Updater]successfully extract file " << stat.m_file_index << " to "<<dest_file;
-            }
-            catch (const std::exception& e)
-            {
-                // ensure the zip archive is closed and rethrow the exception
-                close_zip_reader(&archive);
-                BOOST_LOG_TRIVIAL(error) << "[Orca Updater]Archive read exception:"<<e.what();
-                return false;
-            }
-        }
-        else {
-            BOOST_LOG_TRIVIAL(warning) << "[Orca Updater]Unzip: read file stat failed";
-        }
-    }
-    close_zip_reader(&archive);
-
-	return true;
+    const std::string parent_path = (!dest_path.empty() ? dest_path : source_path.parent_path()).string();
+    return extract_archive_confined(source_path.string(), parent_path);
 }
 
 // Remove a leftover partial archive for the vendor about to be synchronized.

tests/libslic3r/CMakeLists.txt

@@ -40,6 +40,7 @@ add_executable(${_TEST_NAME}_tests
     test_lay_on_face.cpp
     test_model.cpp
     test_utils.cpp
+    test_miniz_extension.cpp
     test_timeutils.cpp
     test_voronoi.cpp
     test_wipe_tower_estimate.cpp

tests/libslic3r/test_miniz_extension.cpp

@@ -0,0 +1,194 @@
+#include <catch2/catch_all.hpp>
+
+#include "libslic3r/miniz_extension.hpp"
+
+#include "test_utils.hpp"
+
+#include <boost/filesystem.hpp>
+
+#include <algorithm>
+#include <fstream>
+#include <iterator>
+#include <string>
+#include <utility>
+#include <vector>
+
+using namespace Slic3r;
+namespace fs = boost::filesystem;
+
+namespace {
+
+void write_zip(const fs::path &zip_file, const std::vector<std::pair<std::string, std::string>> &entries)
+{
+    mz_zip_archive zip;
+    mz_zip_zero_struct(&zip);
+    REQUIRE(open_zip_writer(&zip, zip_file.string()));
+    for (const auto &[name, content] : entries)
+        REQUIRE(mz_zip_writer_add_mem(&zip, name.c_str(), content.data(), content.size(), MZ_DEFAULT_COMPRESSION));
+    REQUIRE(mz_zip_writer_finalize_archive(&zip));
+    REQUIRE(close_zip_writer(&zip));
+}
+
+// miniz refuses to write a name starting with '/', so write a placeholder of the same length and patch it in place.
+void rename_entry(const fs::path &zip_file, const std::string &from, const std::string &to)
+{
+    REQUIRE(from.size() == to.size());
+    std::string bytes;
+    {
+        std::ifstream in(zip_file.string(), std::ios::binary);
+        bytes.assign(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
+    }
+    size_t count = 0;
+    for (size_t pos = bytes.find(from); pos != std::string::npos; pos = bytes.find(from, pos + to.size()), ++count)
+        bytes.replace(pos, from.size(), to);
+    // Once in the local header and once in the central directory.
+    REQUIRE(count == 2);
+    std::ofstream out(zip_file.string(), std::ios::binary | std::ios::trunc);
+    out << bytes;
+}
+
+std::vector<std::string> list_dir(const fs::path &dir)
+{
+    std::vector<std::string> names;
+    for (const fs::directory_entry &entry : fs::directory_iterator(dir))
+        names.push_back(entry.path().filename().string());
+    std::sort(names.begin(), names.end());
+    return names;
+}
+
+std::string read_file(const fs::path &file)
+{
+    std::ifstream in(file.string(), std::ios::binary);
+    return std::string(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
+}
+
+} // namespace
+
+TEST_CASE("Confined extraction writes a well-formed archive under the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+    write_zip(zip_file, {{"vendor/", ""}, {"vendor/machine/", ""}, {"vendor.json", "{\"a\":1}"}, {"vendor/machine/printer.json", "{\"b\":2}"}});
+
+    REQUIRE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK(fs::is_directory(target / "vendor"));
+    CHECK(read_file(target / "vendor.json") == "{\"a\":1}");
+    CHECK(read_file(target / "vendor" / "machine" / "printer.json") == "{\"b\":2}");
+}
+
+TEST_CASE("Confined extraction rejects an archive with an entry outside the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    const std::string escaping_entry = GENERATE(std::string("../escape.txt"), std::string("..\\escape.txt"),
+                                                std::string("sub/../../escape.txt"), std::string("C:/escape.txt"),
+                                                std::string("C:escape.txt"), std::string("\\escape.txt"));
+    // The normal entry comes first so a per-entry check would already have written it.
+    write_zip(zip_file, {{"normal.json", "{}"}, {escaping_entry, "escaped"}});
+
+    CAPTURE(escaping_entry);
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction rejects an archive with an absolute entry name", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    const std::string absolute    = (tmp.path() / "escape.txt").generic_string();
+    const std::string placeholder = "#" + absolute.substr(1);
+    write_zip(zip_file, {{"normal.json", "{}"}, {placeholder, "escaped"}});
+    rename_entry(zip_file, placeholder, absolute);
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction rejects a directory entry outside the target directory", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+    write_zip(zip_file, {{"vendor/", ""}, {"../outside/", ""}});
+
+    CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+    CHECK_FALSE(fs::exists(tmp.path() / "outside"));
+    CHECK(fs::is_empty(target));
+}
+
+TEST_CASE("Confined extraction validates zero-size entries like any other", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    SECTION("an empty file inside the target does not fail the archive") {
+        write_zip(zip_file, {{"empty.json", ""}, {"vendor.json", "{}"}});
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK(read_file(target / "vendor.json") == "{}");
+    }
+    SECTION("an empty file outside the target rejects the archive") {
+        write_zip(zip_file, {{"vendor.json", "{}"}, {"../escape.txt", ""}});
+        CHECK_FALSE(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(tmp.path() / "escape.txt"));
+        CHECK(fs::is_empty(target));
+    }
+}
+
+TEST_CASE("Confined extraction writes nothing outside the target for Windows-specific name forms", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    fs::create_directories(target);
+
+    // Windows strips trailing dots and spaces and maps device names; whether these extract depends on the
+    // platform, but none of them may land beside the target.
+    const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("..."), std::string(".. "),
+                                      std::string(".. /escape.txt"), std::string(".../escape.txt"), std::string("CON"),
+                                      std::string("sub/NUL.txt"), std::string("C:escape.txt"));
+    write_zip(zip_file, {{name, "payload"}});
+
+    CAPTURE(name);
+    extract_archive_confined(zip_file.string(), target.string());
+    CHECK(list_dir(tmp.path()) == std::vector<std::string>{"bundle.zip", "cache"});
+}
+
+#ifndef _WIN32
+TEST_CASE("Confined extraction replaces a symlink at the destination instead of writing through it", "[MinizExtension]")
+{
+    ScopedTemporaryDir tmp;
+    const fs::path     zip_file = tmp.path() / "bundle.zip";
+    const fs::path     target   = tmp.path() / "cache";
+    const fs::path     outside  = tmp.path() / "outside";
+    fs::create_directories(target);
+    fs::create_directories(outside);
+    write_zip(zip_file, {{"vendor.json", "{\"a\":1}"}});
+
+    SECTION("a dangling symlink") {
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        CHECK(extract_archive_confined(zip_file.string(), target.string()));
+        CHECK_FALSE(fs::exists(outside / "vendor.json"));
+        CHECK_FALSE(fs::is_symlink(fs::symlink_status(target / "vendor.json")));
+        CHECK(read_file(target / "vendor.json") == "{\"a\":1}");
+    }
+    SECTION("a symlink to an existing file") {
+        { std::ofstream((outside / "vendor.json").string()) << "original"; }
+        fs::create_symlink(outside / "vendor.json", target / "vendor.json");
+        extract_archive_confined(zip_file.string(), target.string());
+        CHECK(read_file(outside / "vendor.json") == "original");
+    }
+}
+#endif

tests/libslic3r/test_utils.cpp

@@ -153,64 +153,70 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha
     }
 }
 
-TEST_CASE("is_absolute_path_within_root accepts only entries inside the root", "[utils]") {
-    namespace fs = boost::filesystem;
-    ScopedTemporaryDir outer;
-    const fs::path root = outer.path() / "Auxiliaries";
-    fs::create_directories(root / "Others");
-    const fs::path inside = root / "Others" / "note.txt";
-    const fs::path outside = outer.path() / "secret.txt";
-    std::ofstream(inside.string()) << "inside";
-    std::ofstream(outside.string()) << "outside";
+TEST_CASE("is_path_within_root accepts a root given with a trailing separator", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string root = tmp.path().string();
+    const std::string with_separator = GENERATE_COPY(root + "/", root + std::string(1, static_cast<char>(boost::filesystem::path::preferred_separator)));
 
-    SECTION("a file inside the root") {
-        REQUIRE(is_absolute_path_within_root(inside, root));
+    CAPTURE(with_separator);
+    CHECK(is_path_within_root("vendor.json", with_separator));
+    CHECK(is_path_within_root("vendor/machine/printer.json", with_separator));
+    CHECK_FALSE(is_path_within_root("../vendor.json", with_separator));
+}
+
+TEST_CASE("is_path_within_root treats Windows-specific name forms the same on every platform", "[utils]") {
+    ScopedTemporaryDir tmp;
+
+    SECTION("names ending in dots or spaces stay inside the root") {
+        const std::string name = GENERATE(std::string("name."), std::string("name "), std::string("dir./file.json"), std::string("dir /file.json"));
+        CAPTURE(name);
+        CHECK(is_path_within_root(name, tmp.path()));
     }
-    SECTION("a path inside the root whose file does not exist yet") {
-        REQUIRE(is_absolute_path_within_root(root / "Others" / "missing.txt", root));
-    }
-    SECTION("the root itself") {
-        REQUIRE_FALSE(is_absolute_path_within_root(root, root));
-    }
-    SECTION("a parent-directory escape spelled under the root") {
-        REQUIRE_FALSE(is_absolute_path_within_root(root / "Others" / ".." / ".." / "secret.txt", root));
-    }
-    SECTION("an absolute path elsewhere") {
-        REQUIRE_FALSE(is_absolute_path_within_root(outside, root));
-    }
-    SECTION("a sibling directory sharing the root's name as a prefix") {
-        const fs::path sibling = outer.path() / "Auxiliaries2" / "note.txt";
-        REQUIRE_FALSE(is_absolute_path_within_root(sibling, root));
-    }
-    SECTION("a relative path") {
-        REQUIRE_FALSE(is_absolute_path_within_root(fs::path("Others") / "note.txt", root));
-    }
-    SECTION("an empty path") {
-        REQUIRE_FALSE(is_absolute_path_within_root(fs::path(), root));
+    SECTION("drive-relative names are rejected") {
+        const std::string name = GENERATE(std::string("C:x"), std::string("c:x/y.json"), std::string("C:"));
+        CAPTURE(name);
+        CHECK_FALSE(is_path_within_root(name, tmp.path()));
     }
+}
+
+TEST_CASE("is_symlink_target_within_root accepts relative targets that stay inside the root", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const auto [link, target] = GENERATE(std::make_pair(std::string("Versions/Current"), std::string("A")),
+                                         std::make_pair(std::string("Foo.framework/Foo"), std::string("Versions/Current/Foo")),
+                                         std::make_pair(std::string("libfoo.so"), std::string("libfoo.so.1")),
+                                         std::make_pair(std::string("a/b/link"), std::string("c/d")));
+    CAPTURE(link, target);
+    CHECK(is_symlink_target_within_root(link, target, tmp.path()));
+}
+
+TEST_CASE("is_symlink_target_within_root rejects absolute targets and targets that climb out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const std::string outside = (tmp.path().parent_path() / "outside").generic_string();
+    const auto [link, target] = GENERATE_COPY(std::make_pair(std::string("sub/link"), outside),
+                                              std::make_pair(std::string("sub/link"), std::string("/etc/passwd")),
+                                              std::make_pair(std::string("sub/link"), std::string("\\outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:/outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("C:outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("")),
+                                              std::make_pair(std::string("link"), std::string("..")),
+                                              std::make_pair(std::string("link"), std::string("../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("../../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("x/../../../outside")),
+                                              std::make_pair(std::string("sub/link"), std::string("..\\..\\outside")));
+    CAPTURE(link, target);
+    CHECK_FALSE(is_symlink_target_within_root(link, target, tmp.path()));
+}
+
 #ifndef _WIN32
-    // Creating symlinks on Windows needs elevated rights or developer mode.
-    SECTION("a symlink inside the root that points outside") {
-        const fs::path link = root / "Others" / "link.txt";
-        fs::create_symlink(outside, link);
-        REQUIRE_FALSE(is_absolute_path_within_root(link, root));
-    }
+TEST_CASE("is_symlink_target_within_root rejects a target that passes through a symlink leading out", "[utils]") {
+    ScopedTemporaryDir tmp;
+    const boost::filesystem::path root    = tmp.path() / "root";
+    const boost::filesystem::path outside = tmp.path() / "outside";
+    boost::filesystem::create_directories(root);
+    boost::filesystem::create_directories(outside);
+    boost::filesystem::create_symlink(outside, root / "out");
+
+    CHECK_FALSE(is_symlink_target_within_root("link", "out/lib.so", root));
+    CHECK(is_symlink_target_within_root("link", "in/lib.so", root));
+}
 #endif
-}
-
-TEST_CASE("is_executable_file_name flags attachments that would run as programs", "[utils]") {
-    const std::string executable = GENERATE(as<std::string>{},
-        "setup.exe", "SETUP.EXE", "Manual.pdf.exe", "run.bat", "start.cmd", "shortcut.lnk", "site.url", "script.ps1",
-        "macro.vbs", "tool.jar", "script.py", "Install.command", "Tool.app", "installer.pkg", "install.sh",
-        "launcher.desktop", "Printer.AppImage", "setup.exe.", "setup.exe ", "setup.exe. .", ".exe");
-    INFO(executable);
-    CHECK(is_executable_file_name(executable));
-}
-
-TEST_CASE("is_executable_file_name leaves documents and models alone", "[utils]") {
-    const std::string document = GENERATE(as<std::string>{},
-        "Manual.pdf", "BOM.xlsx", "notes.txt", "photo.JPG", "assembly.step", "part.stl", "project.3mf", "readme",
-        "exe", "setup.exe.pdf", "", "...", "dir.exe/readme");
-    INFO(document);
-    CHECK_FALSE(is_executable_file_name(document));
-}