Open Project Attachments Through One Guarded Helper

The Edit Project Info view launched attachments directly, without the
checks the project page has. Both now call
desktop_open_project_attachment, which checks that the file is inside
the auxiliary directory, asks for confirmation where needed and then
opens it.

The auxiliary root was built through encode_path, which returns code
page bytes on Windows, while boost::filesystem reads a narrow string as
UTF-8. With a non-ASCII temporary directory the root never matched and
no attachment opened. It is now built from the UTF-8 path directly.

The list of program extensions could not be kept complete and let
unknown types open without a prompt. It is replaced by
is_safe_to_open_file_name, a list of plain document, image, model and
video types that open directly. Everything else asks first.

resources/web/model/model.js

@@ -214,9 +214,9 @@ function ShowModelInfo( pModel )
 	
 	SendWXDebugInfo("Model Name:  "+sModelName);
 	
-	$('#ModelName').html(sModelName);
+	$('#ModelName').text(sModelName);
 	$('#ModelName').attr('title',sModelName);
-    $('#ModelAuthorName').html(sModelAuthor);
+    $('#ModelAuthorName').text(sModelAuthor);
 	
 	switch(UploadType)
 	{
@@ -268,7 +268,7 @@ function ShowModelInfo( pModel )
 			break;
 	}
 	
-	$('#Model_Desc').html( html_decode(sModelDesc) );
+	$('#Model_Desc').empty().append( SanitizeDescHtml( html_decode(sModelDesc) ) );
 			
 	let ModelPreviewList=pModel.preview_img;				
     let TotalPreview=ModelPreviewList.length;
@@ -281,16 +281,15 @@ function ShowModelInfo( pModel )
 	
     if(TotalPreview>0)
 	{
-		let htmlPreview='';
+		$('#ModelPreviewList').empty();
 		for(let pn=0;pn<TotalPreview;pn++)			
 		{	
 			//let FTmpPath=decodeURIComponent(ModelPreviewList[pn]);
 			let FTmpPath=ModelPreviewList[pn]['filepath'];
 			
-			htmlPreview+='<div class="swiper-slide"><img class="Model_PrevImg" src="'+FTmpPath+'" /></div>';
+			$('#ModelPreviewList').append( $('<div class="swiper-slide"></div>').append( $('<img class="Model_PrevImg" />').attr('src',FTmpPath) ) );
 		}
 			
-	    $('#ModelPreviewList').html(htmlPreview);
 		$('#Model_Preview_Image').viewer({
 			title: false,
 		    fullsreen: false,
@@ -410,7 +409,8 @@ function ConstructFileHtml( ID, pItem )
 {
 	let fTotal=pItem.length;
 	
-	let strHtml='';
+	let pBoard=$('#'+ID+'  .FileListBoard');
+	pBoard.empty();
 	for( let f=0;f<fTotal;f++ )
 	{
 		let pOne=pItem[f];
@@ -443,39 +443,139 @@ function ConstructFileHtml( ID, pItem )
 			ImgPath='img/default.png';			
 		}		
 			
-		//Add html
+		//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>');
 		if( strClass!='ImageIcon' )
 		{
-		strHtml+='<div class="FileItem">'+
+			pMenu.on('click', function(){ OnClickOpenFile(tPath); });
-				 '	<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;
 			
-		strHtml+='<div class="FileItem">'+
+			pIconImg.attr('id',TmpImgID);
-				 '	<div class="'+strClass+'"><img id="'+TmpImgID+'" src="'+ImgPath+'" /></div>'+
+			pMenu.on('click', function(){ OnClickOpenImage(TmpImgID); });
-				 '	<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==========
@@ -483,10 +583,10 @@ function ShowProfilelInfo( pProfile )
 	let sProfileAuthor=decodeURIComponent(pProfile.author);
 	let sProfileDesc=decodeURIComponent(pProfile.description);
 	
-	$('#ProfileName').html(sProfileName);
+	$('#ProfileName').text(sProfileName);
-    $('#ProfileAuthor').html(sProfileAuthor);
+    $('#ProfileAuthor').text(sProfileAuthor);
 		
-	$('#Profile_Desc').html( html_decode(sProfileDesc) );
+	$('#Profile_Desc').empty().append( SanitizeDescHtml( html_decode(sProfileDesc) ) );
 			
 	let ProfilePreviewList=pProfile.preview_img;				
     let TotalPreview=ProfilePreviewList.length;
@@ -499,15 +599,14 @@ function ShowProfilelInfo( pProfile )
 	
     if(TotalPreview>0)
 	{
-		let htmlPreview='';
+		$('#ProfilePreviewList').empty();
 		for(let pn=0;pn<TotalPreview;pn++)			
 		{	
 			let FTmpPath=ProfilePreviewList[pn]['filepath'];
 			
-			htmlPreview+='<div class="swiper-slide"><img class="Model_PrevImg" src="'+FTmpPath+'" /></div>';
+			$('#ProfilePreviewList').append( $('<div class="swiper-slide"></div>').append( $('<img class="Model_PrevImg" />').attr('src',FTmpPath) ) );
 		}
 			
-		$('#ProfilePreviewList').html(htmlPreview);
 		$('#Profile_Preview_Image').viewer({
 			title: false,
 		    fullsreen: false,

src/libslic3r/Format/bbs_3mf.cpp

@@ -1629,7 +1629,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
         }
         while (it != m_plater_data.end())
         {
-            if (it->first <= 0 || static_cast<size_t>(it->first) > m_plater_data.size())
+            if (it->first > m_plater_data.size())
             {
                 add_error("invalid plate index");
                 return false;
@@ -2312,7 +2312,7 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result)
         }
         while (it != m_plater_data.end())
         {
-            if (it->first <= 0 || static_cast<size_t>(it->first) > m_plater_data.size())
+            if (it->first > m_plater_data.size())
             {
                 add_error("invalid plate index");
                 return false;

src/libslic3r/Model.cpp

@@ -3702,11 +3702,7 @@ void FacetsAnnotation::set_triangle_from_string(int triangle_id, const std::stri
             m_data.bitstream.insert(m_data.bitstream.end(), bool(dec & (1 << i)));
     }
 
-    if (!m_data.update_used_states(bitstream_start_idx)) {
+    m_data.update_used_states(bitstream_start_idx);
-        BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": dropping malformed paint data of triangle " << triangle_id;
-        m_data.bitstream.resize(bitstream_start_idx);
-        m_data.triangles_to_split.pop_back();
-    }
 }
 
 bool FacetsAnnotation::equals(const FacetsAnnotation &other) const

src/libslic3r/TriangleSelector.cpp

@@ -1778,13 +1778,6 @@ TriangleSelector::TriangleSplittingData TriangleSelector::serialize() const {
     return out.data;
 }
 
-// A split code keeps the split side (one split) or the kept side (two splits) in its upper two
-// bits, where 3 is not a side. The value is ignored for a three-side split.
-static bool split_code_valid(int code)
-{
-    return (code & 0b11) == 3 || (code >> 2) != 3;
-}
-
 void TriangleSelector::deserialize(const TriangleSplittingData &data,
                                    bool                         needs_reset,
                                    EnforcerBlockerType          max_ebt,
@@ -1819,12 +1812,11 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
 
     for (auto [triangle_id, ibit] : data.triangles_to_split) {
         assert(triangle_id < int(m_triangles.size()));
-        // Set when the bitstream runs out or holds an impossible split before this triangle's tree is complete.
+        assert(ibit < int(data.bitstream.size()));
-        bool corrupt = false;
+        auto next_nibble = [&data, &ibit = ibit]() {
-        auto next_nibble = [&data, &ibit = ibit, &corrupt]() {
             int n = 0;
-            if (! data.read_nibble(ibit, n))
+            for (int i = 0; i < 4; ++ i)
-                corrupt = true;
+                n |= data.bitstream[ibit ++] << i;
             return n;
         };
         // Decode a leaf state stored behind the "11" prefix: one nibble of (state-3) for states
@@ -1843,10 +1835,6 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             bool is_split = num_of_children != 0;
             // Only valid if not is_split.
             auto state = is_split ? EnforcerBlockerType::NONE : ((code & 0b1100) == 0b1100 ? decode_leaf_state() : EnforcerBlockerType(code >> 2));
-            if (is_split && ! split_code_valid(code))
-                corrupt = true;
-            if (corrupt)
-                break;
 
             // BBS
             if (state == to_delete_filament)
@@ -1861,7 +1849,7 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             }
 
             // Only valid if is_split.
-            int special_side = num_of_split_sides == 3 ? 0 : code >> 2;
+            int special_side = code >> 2;
 
             // Take care of the first iteration separately, so handling of the others is simpler.
             if (parents.empty()) {
@@ -1916,55 +1904,47 @@ void TriangleSelector::deserialize(const TriangleSplittingData &data,
             if (parents.empty())
                 break;
         }
-
-        if (corrupt) {
-            // Every split above allocated all of its children, so the partial tree unwinds cleanly.
-            BOOST_LOG_TRIVIAL(warning) << __FUNCTION__ << ": malformed paint data, dropping paint of triangle " << triangle_id;
-            undivide_triangle(triangle_id);
-            m_triangles[triangle_id].set_state(EnforcerBlockerType::NONE);
-        }
     }
 }
 
-bool TriangleSelector::TriangleSplittingData::update_used_states(const size_t bitstream_start_idx) {
+void TriangleSelector::TriangleSplittingData::update_used_states(const size_t bitstream_start_idx) {
-    int      ibit   = static_cast<int>(bitstream_start_idx);
+    assert(bitstream_start_idx < this->bitstream.size());
-    uint64_t states = 0;
+    assert(!this->bitstream.empty() && this->bitstream.size() != bitstream_start_idx);
-    do {
+    assert((this->bitstream.size() - bitstream_start_idx) % 4 == 0);
-        // Walk one triangle's tree depth-first, counting the nodes still to be read; a split node adds its children.
-        for (int pending_nodes = 1; pending_nodes > 0; --pending_nodes) {
-            int code;
-            if (!this->read_nibble(ibit, code))
-                return false;
 
-            if (const int num_of_split_sides = code & 0b11; num_of_split_sides != 0) {
+    if (this->bitstream.empty() || this->bitstream.size() == bitstream_start_idx)
-                if (!split_code_valid(code))
+        return;
-                    return false;
-                pending_nodes += num_of_split_sides + 1;
-                continue;
-            }
 
-            int facet_state = code >> 2;
+    size_t nibble_idx = bitstream_start_idx;
-            if (facet_state == 0b11) {
+
-                // Leaf behind the "11" prefix: one nibble of (state-3), or 0b1111 + (state-18).
+    auto read_next_nibble = [&data_bitstream = std::as_const(this->bitstream), &nibble_idx]() -> uint8_t {
-                int nibble;
+        assert(nibble_idx + 3 < data_bitstream.size());
-                if (!this->read_nibble(ibit, nibble))
+        uint8_t code = 0;
-                    return false;
+        for (size_t bit_idx = 0; bit_idx < 4; ++bit_idx)
-                facet_state = nibble + 3;
+            code |= data_bitstream[nibble_idx++] << bit_idx;
-                if (nibble == 0b1111) {
+        return code;
-                    if (!this->read_nibble(ibit, nibble))
+    };
-                        return false;
+
-                    facet_state = nibble + 18;
+    while (nibble_idx < this->bitstream.size()) {
-                }
+        const uint8_t code = read_next_nibble();
-            }
+
-            states |= uint64_t(1) << facet_state;
+        if (const bool is_split = (code & 0b11) != 0; is_split)
+            continue;
+
+        uint8_t facet_state;
+        if ((code & 0b1100) == 0b1100) {
+            // Leaf behind the "11" prefix: one nibble of (state-3), or 0b1111 + (state-18).
+            const uint8_t nibble = read_next_nibble();
+            facet_state = nibble == 0b1111 ? uint8_t(read_next_nibble() + 18) : uint8_t(nibble + 3);
+        } else {
+            facet_state = code >> 2;
         }
-    } while (static_cast<size_t>(ibit) < this->bitstream.size());
+        assert(facet_state < this->used_states.size());
+        if (facet_state >= this->used_states.size())
+            continue;
 
-    // The leaf encoding tops out at state 33, so every state fits the 64-bit mask.
+        this->used_states[facet_state] = true;
-    for (size_t state_idx = 0; state_idx < std::min<size_t>(this->used_states.size(), 64); ++state_idx)
+    }
-        if (states & (uint64_t(1) << state_idx))
-            this->used_states[state_idx] = true;
-    return true;
 }
 
 // Lightweight variant of deserialization, which only tests whether a face of test_state exists.
@@ -1976,12 +1956,11 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
 
     for (const TriangleBitStreamMapping &triangle_id_and_ibit : data.triangles_to_split) {
         int ibit = triangle_id_and_ibit.bitstream_start_idx;
-        // Stop reading a triangle whose stream is truncated.
+        assert(ibit < int(data.bitstream.size()));
-        bool truncated = false;
+        auto next_nibble = [&data, &ibit = ibit]() {
-        auto next_nibble = [&data, &ibit = ibit, &truncated]() {
             int n = 0;
-            if (! data.read_nibble(ibit, n))
+            for (int i = 0; i < 4; ++ i)
-                truncated = true;
+                n |= data.bitstream[ibit ++] << i;
             return n;
         };
         // < 0 -> negative of a number of children
@@ -1999,8 +1978,6 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
         };
 
         int state = num_children_or_state();
-        if (truncated)
-            continue;
         if (state < 0) {
             // Root is split.
             parents_children.clear();
@@ -2008,8 +1985,6 @@ bool TriangleSelector::has_facets(const TriangleSplittingData &data, const Enfor
             do {
                 if (-- parents_children.back() >= 0) {
                     int state = num_children_or_state();
-                    if (truncated)
-                        break;
                     if (state < 0)
                         // Child is split.
                         parents_children.emplace_back(- state);

src/libslic3r/TriangleSelector.hpp

@@ -297,20 +297,8 @@ public:
             std::fill(used_states.begin(), used_states.end(), false);
         }
 
-        // Update used states from the triangle trees stored between bitstream_start_idx and the end of the bitstream.
+        // Update used states based on the bitstream. It just iterated over the bitstream from the bitstream_start_idx till the end.
-        // Returns false and leaves used states untouched if a tree is truncated or malformed.
+        void update_used_states(size_t bitstream_start_idx);
-        bool update_used_states(size_t bitstream_start_idx);
-
-        // Read the 4-bit code at bit index ibit (LSB first) and advance ibit past it.
-        // Returns false without advancing when fewer than 4 bits remain.
-        bool read_nibble(int &ibit, int &nibble) const {
-            if (ibit < 0 || static_cast<size_t>(ibit) + 4 > bitstream.size())
-                return false;
-            nibble = 0;
-            for (int i = 0; i < 4; ++i)
-                nibble |= static_cast<int>(bitstream[ibit++]) << i;
-            return true;
-        }
 
     private:
         friend class cereal::access;

src/libslic3r/Utils.hpp

@@ -260,6 +260,12 @@ 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 a file with this name is of a type that the desktop opens as plain content, so it cannot run code.
+// Anything unknown is not safe.
+extern bool is_safe_to_open_file_name(const std::string &file_name);
 
 // Orca: custom protocal support utils
 inline bool is_orca_open(const std::string& url) { return boost::starts_with(url, "orcaslicer://open"); }

src/libslic3r/utils.cpp

@@ -70,6 +70,7 @@
 #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>
@@ -1112,6 +1113,28 @@ 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)
+{
+    const boost::filesystem::path rel = path.lexically_relative(root);
+    return !rel.empty() && rel != "." && is_path_within_root(rel.string(), root);
+}
+
+bool is_safe_to_open_file_name(const std::string &file_name)
+{
+    // Formats that cannot carry macros or scripts. Legacy and OpenDocument office files, HTML and SVG are left out on purpose.
+    static const std::vector<std::string> safe_extensions = {
+        "jpg", "jpeg", "jfif", "pjpeg", "pjp", "png", "gif", "bmp", "webp", "tif", "tiff",
+        "pdf", "txt", "md", "csv", "docx", "xlsx", "pptx",
+        "stl", "obj", "3mf", "amf", "ply", "step", "stp", "iges", "igs", "dxf",
+        "mp4", "mov", "webm"};
+    // The name must end in the extension itself: Windows drops trailing dots and spaces and reads ':' as a stream separator.
+    const size_t dot = file_name.find_last_of('.');
+    if (dot == std::string::npos || file_name.find_first_of("/\\:") != std::string::npos)
+        return false;
+    const std::string extension = boost::algorithm::to_lower_copy(file_name.substr(dot + 1));
+    return std::find(safe_extensions.begin(), safe_extensions.end(), extension) != safe_extensions.end();
+}
+
 bool is_img_file(const std::string &path)
 {
 	return boost::iends_with(path, ".png") || boost::iends_with(path, ".svg");

src/slic3r/GUI/Auxiliary.cpp

@@ -428,7 +428,7 @@ void AuFile::on_dclick(wxMouseEvent &evt)
     if (m_type == AddFileButton)
         return;
     else
-        wxLaunchDefaultApplication(m_file_path.wstring(), 0);
+        desktop_open_project_attachment(this, m_file_path);
 }
 
 void AuFile::on_mouse_left_up(wxMouseEvent &evt)

src/slic3r/GUI/GUI.cpp

@@ -27,11 +27,13 @@
 
 #include "AboutDialog.hpp"
 #include "MsgDialog.hpp"
+#include "Plater.hpp"
 #include "format.hpp"
 
 #include "WebUserLoginDialog.hpp"
 
 #include "libslic3r/Print.hpp"
+#include "libslic3r/Utils.hpp"
 
 namespace Slic3r {
 
@@ -635,4 +637,26 @@ void desktop_open_any_folder( const std::string& path )
 }
 
 
+bool desktop_open_project_attachment(wxWindow *parent, const boost::filesystem::path &path)
+{
+    // The auxiliary path is UTF-8, which is what boost::filesystem reads a narrow string as.
+    const boost::filesystem::path aux_root(wxGetApp().plater()->model().get_auxiliary_file_temp_path());
+    boost::system::error_code ec;
+    if (!is_absolute_path_within_root(path, aux_root) || !boost::filesystem::is_regular_file(path, ec))
+        return false;
+
+    // Attachments come with the project and carry no download mark, so the desktop would open them without a warning.
+    if (!is_safe_to_open_file_name(path.filename().string())) {
+        MessageDialog dlg(parent,
+                          wxString::Format(_L("\"%s\" is not a plain document, image or model file. Opening it may run it as a "
+                                              "program or script 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);
+        if (dlg.ShowModal() != wxID_YES)
+            return false;
+    }
+    return wxLaunchDefaultApplication(from_path(path), 0);
+}
+
 } }

src/slic3r/GUI/GUI.hpp

@@ -83,6 +83,9 @@ extern void about();
 extern void desktop_open_datadir_folder();
 // Ask the destop to open one folder
 extern void desktop_open_any_folder(const std::string& path);
+// Ask the desktop to open a file from the project's auxiliary directory, after a confirmation
+// unless its type is known to be plain content. Returns false if the file was not opened.
+extern bool desktop_open_project_attachment(wxWindow *parent, const boost::filesystem::path &path);
 } // namespace GUI
 } // namespace Slic3r
 

src/slic3r/GUI/Project.cpp

@@ -293,10 +293,8 @@ void ProjectPanel::OnScriptMessage(wxWebViewEvent& evt)
             if (!accessory_path.empty()) {
                 std::string decode_path = wxGetApp().url_decode(accessory_path.ToStdString());
                 fs::path path(decode_path);
-
+                if (!desktop_open_project_attachment(this, path))
-                if (fs::exists(path)) {
+                    BOOST_LOG_TRIVIAL(warning) << "open_3mf_accessory: not opening " << decode_path;
-                    wxLaunchDefaultApplication(path.wstring(), 0);
-                }
             }
         }
         else if (strCmd == "request_3mf_info") {

tests/libslic3r/test_3mf.cpp

@@ -19,7 +19,6 @@
 #include <boost/filesystem/operations.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <algorithm>
-#include <functional>
 
 #include <catch2/catch_tostring.hpp>
 #include <Eigen/Core>
@@ -185,13 +184,16 @@ static bool read_cad_recipe_entry(const std::string& path, std::string& out,
     return found;
 }
 
-// Rewrites the archive at `path`, letting `edit` change the name or data of each entry; returns
+// Rewrites the archive at `path` with the recipe entry back under the name it had before the
-// whether `edit` reported a change for any of them. miniz cannot edit in place and
+// rename, which is what every project saved by an earlier build looks like on disk. Generated
-// open_zip_writer truncates, so the entries are held across the switch.
+// rather than checked in because a whole project archive is not frozen evidence the way a bare
-static bool rewrite_3mf_entries(const std::string& path, const std::function<bool(std::string& name, std::string& data)>& edit)
+// recipe blob is -- it has to be whatever today's exporter writes, with only the name aged.
+// miniz cannot rename in place and open_zip_writer truncates, so the entries are held across
+// the switch.
+static void rename_cad_recipe_entry_to_legacy(const std::string& path)
 {
     std::vector<std::pair<std::string, std::string>> entries;
-    bool changed = false;
+    bool renamed = false;
     {
         mz_zip_archive zip;
         mz_zip_zero_struct(&zip);
@@ -206,140 +208,22 @@ static bool rewrite_3mf_entries(const std::string& path, const std::function<boo
             std::string data((size_t) st.m_uncomp_size, '\0');
             if (st.m_uncomp_size > 0)
                 REQUIRE(mz_zip_reader_extract_to_mem(&zip, i, data.data(), data.size(), 0));
-            changed |= edit(name, data);
+            if (boost::algorithm::iequals(name, CAD_RECIPE_ENTRY)) {
+                name    = LEGACY_CAD_RECIPE_ENTRY;
+                renamed = true;
+            }
             entries.emplace_back(std::move(name), std::move(data));
         }
         close_zip_reader(&zip);
     }
+    // Without this the scenario would degrade silently into re-testing the new name if the
+    // exporter's constant ever moved again: every load below would still pass.
+    REQUIRE(renamed);
 
     Zipper out(path);
     for (const auto& e : entries)
         out.add_entry(e.first, e.second.data(), e.second.size());
     out.finalize();
-    return changed;
-}
-
-// Rewrites the archive at `path` with the recipe entry back under the name it had before the
-// rename, which is what every project saved by an earlier build looks like on disk. Generated
-// rather than checked in because a whole project archive is not frozen evidence the way a bare
-// recipe blob is -- it has to be whatever today's exporter writes, with only the name aged.
-static void rename_cad_recipe_entry_to_legacy(const std::string& path)
-{
-    const bool renamed = rewrite_3mf_entries(path, [](std::string& name, std::string&) {
-        if (!boost::algorithm::iequals(name, CAD_RECIPE_ENTRY))
-            return false;
-        name = LEGACY_CAD_RECIPE_ENTRY;
-        return true;
-    });
-    // Without this the scenario would degrade silently into re-testing the new name if the
-    // exporter's constant ever moved again: every load below would still pass.
-    REQUIRE(renamed);
-}
-
-// Replaces the first occurrence of `from` in any entry whose name ends with `suffix`.
-static bool replace_in_3mf_entry(const std::string& path, const std::string& suffix, const std::string& from, const std::string& to)
-{
-    bool replaced = false;
-    rewrite_3mf_entries(path, [&](std::string& name, std::string& data) {
-        if (replaced || !boost::algorithm::ends_with(name, suffix))
-            return false;
-        if (const size_t pos = data.find(from); pos != std::string::npos) {
-            data.replace(pos, from.size(), to);
-            replaced = true;
-        }
-        return replaced;
-    });
-    return replaced;
-}
-
-// Stores a one-plate project holding a cube whose first two facets are painted Extruder2 and
-// Extruder3, which the exporter writes as paint_color="8" and paint_color="0C".
-static void store_painted_cube(const std::string& path)
-{
-    Model        model;
-    ModelObject* object = model.add_object();
-    ModelVolume* volume = object->add_volume(make_cube(10., 10., 10.));
-    object->add_instance();
-    {
-        TriangleSelector selector(volume->mesh());
-        selector.set_facet(0, EnforcerBlockerType::Extruder2);
-        selector.set_facet(1, EnforcerBlockerType::Extruder3);
-        REQUIRE(volume->mmu_segmentation_facets.set(selector));
-    }
-    ScopedTemporaryDir backup_dir("orca_paint_src");
-    model.set_backup_path(backup_dir.string());
-
-    DynamicPrintConfig cfg;
-    PlateData          plate;
-    plate.plate_index = 0;
-    StoreParams sp;
-    sp.path     = path.c_str();
-    sp.model    = &model;
-    sp.config   = &cfg;
-    sp.strategy = SaveStrategy::Zip64 | SaveStrategy::Silence;
-    sp.plate_data_list.push_back(&plate);
-    REQUIRE(store_bbs_3mf(sp));
-}
-
-// Loads `path` through the BBS importer into `model`, releasing the plates it returns. The
-// importer stages metadata through `backup_dir`, which has to outlive the model.
-static bool load_project(const std::string& path, Model& model, const ScopedTemporaryDir& backup_dir)
-{
-    model.set_backup_path(backup_dir.string());
-    DynamicPrintConfig        config;
-    ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Enable };
-    PlateDataPtrs             plates;
-    std::vector<Preset*>      project_presets;
-    bool   is_bbl_3mf = false, is_orca_3mf = false;
-    Semver file_version;
-    const bool loaded = load_bbs_3mf(path.c_str(), &config, &ctxt, &model, &plates, &project_presets, &is_bbl_3mf,
-                                     &is_orca_3mf, &file_version, nullptr, LoadStrategy::LoadModel | LoadStrategy::LoadConfig);
-    release_PlateData_list(plates);
-    return loaded;
-}
-
-TEST_CASE("A project with a plate id below 1 fails to load", "[3mf][Regression]")
-{
-    const int plate_id = GENERATE(0, -1);
-    INFO("plater_id " << plate_id);
-
-    ScopedTemporaryFile temp(".3mf");
-    store_painted_cube(temp.string());
-    {
-        ScopedTemporaryDir backup_dir("orca_plate_dst");
-        Model              model;
-        REQUIRE(load_project(temp.string(), model, backup_dir));
-    }
-
-    REQUIRE(replace_in_3mf_entry(temp.string(), "model_settings.config", "key=\"plater_id\" value=\"1\"",
-                                 "key=\"plater_id\" value=\"" + std::to_string(plate_id) + "\""));
-    ScopedTemporaryDir backup_dir("orca_plate_dst");
-    Model              model;
-    bool               loaded = true;
-    REQUIRE_NOTHROW(loaded = load_project(temp.string(), model, backup_dir));
-    REQUIRE_FALSE(loaded);
-}
-
-TEST_CASE("A project with malformed paint data loads without the damaged facet", "[3mf][Regression]")
-{
-    ScopedTemporaryFile temp(".3mf");
-    store_painted_cube(temp.string());
-    // Split codes with no children behind them: the stream runs out mid-tree.
-    REQUIRE(replace_in_3mf_entry(temp.string(), ".model", "paint_color=\"8\"", "paint_color=\"FFFFFFFFFFFFFFFF3\""));
-
-    ScopedTemporaryDir backup_dir("orca_paint_dst");
-    Model              model;
-    REQUIRE(load_project(temp.string(), model, backup_dir));
-    REQUIRE(model.objects.size() == 1);
-    const ModelVolume& volume = *model.objects.front()->volumes.front();
-    const auto&        data   = volume.mmu_segmentation_facets.get_data();
-    REQUIRE_FALSE(data.used_states[size_t(EnforcerBlockerType::Extruder2)]);
-    REQUIRE(data.used_states[size_t(EnforcerBlockerType::Extruder3)]);
-
-    TriangleSelector selector(volume.mesh());
-    REQUIRE_NOTHROW(selector.deserialize(data));
-    REQUIRE(selector.num_facets(EnforcerBlockerType::Extruder2) == 0);
-    REQUIRE(selector.num_facets(EnforcerBlockerType::Extruder3) == 1);
 }
 
 // The recipe lives only in the BBS-native backend, because that is the only one that runs:

tests/libslic3r/test_triangle_selector.cpp

@@ -3,8 +3,6 @@
 #include "libslic3r/TriangleSelector.hpp"
 #include "libslic3r/TriangleMesh.hpp"
 
-#include <algorithm>
-
 using namespace Slic3r;
 
 // A sphere gives well over ExtruderMax original facets, so every extruder state can be assigned
@@ -125,77 +123,3 @@ TEST_CASE("Extruder states match the CONST_FILAMENTS hex encoding", "[TriangleSe
     INFO("Hex " << c.hex << " -> extruder " << c.state);
     REQUIRE(TriangleSelector::has_facets(data, EnforcerBlockerType(c.state)));
 }
-
-// Pack 4-bit codes into a bitstream, least significant bit first, in the order the decoder reads them.
-static std::vector<bool> pack_nibbles(const std::vector<int> &nibbles)
-{
-    std::vector<bool> bitstream;
-    for (const int nibble : nibbles)
-        for (int bit = 0; bit < 4; ++bit)
-            bitstream.push_back((nibble >> bit) & 1);
-    return bitstream;
-}
-
-TEST_CASE("A valid paint stream with nested splits round-trips bit for bit", "[TriangleSelector]")
-{
-    const TriangleMesh mesh = test_mesh();
-
-    TriangleSelector::TriangleSplittingData data;
-    data.triangles_to_split.emplace_back(0, 0);
-    // A three-side split whose children, in stream order, are: a one-side split (side 2) into two
-    // leaves, a two-side split (side 1) into leaves of states 20, 0 and 8, then two plain leaves.
-    const std::vector<int> triangle_0 = {0b0011,
-                                         0b1001, 0b1000, 0b0100,
-                                         0b0110, 0b1100, 0b1111, 20 - 18, 0b0000, 0b1100, 8 - 3,
-                                         0b1000,
-                                         0b0100};
-    data.bitstream = pack_nibbles(triangle_0);
-    data.triangles_to_split.emplace_back(5, int(data.bitstream.size()));
-    const std::vector<bool> triangle_5 = pack_nibbles({0b1100, 3 - 3});
-    data.bitstream.insert(data.bitstream.end(), triangle_5.begin(), triangle_5.end());
-    data.reset_used_states();
-    REQUIRE(data.update_used_states(0));
-
-    TriangleSelector restored(mesh);
-    restored.deserialize(data);
-
-    REQUIRE(restored.num_facets(EnforcerBlockerType::Extruder20) == 1);
-    REQUIRE(restored.num_facets(EnforcerBlockerType::Extruder3) == 1);
-    REQUIRE(restored.serialize() == data);
-}
-
-TEST_CASE("A truncated or malformed paint stream drops only the damaged triangle", "[TriangleSelector][Regression]")
-{
-    struct Case { const char *name; std::vector<int> nibbles; };
-    const auto c = GENERATE(values<Case>({
-        {"three-side split missing two children", {0b0011, 0b1000, 0b1000}},
-        {"leaf missing its state nibble",         {0b1100}},
-        {"leaf missing its second state nibble",  {0b1100, 0b1111}},
-        {"splits nested past the end",            {0b0011, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF,
-                                                    0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF, 0xF}},
-        {"one-side split of the nonexistent side 3", {0b1101, 0b1000, 0b1000}},
-    }));
-    INFO(c.name);
-
-    const TriangleMesh mesh = test_mesh();
-    TriangleSelector   intact(mesh);
-    intact.set_facet(0, EnforcerBlockerType::Extruder2);
-
-    // Triangle 0 stays intact, triangle 1 carries the damaged stream.
-    TriangleSelector::TriangleSplittingData data = intact.serialize();
-    data.triangles_to_split.emplace_back(1, int(data.bitstream.size()));
-    const std::vector<bool> damaged = pack_nibbles(c.nibbles);
-    data.bitstream.insert(data.bitstream.end(), damaged.begin(), damaged.end());
-
-    TriangleSelector restored(mesh);
-    REQUIRE_NOTHROW(restored.deserialize(data));
-    // Triangle 1 unwinds completely, so the selector holds exactly the intact paint.
-    REQUIRE(restored.serialize() == intact.serialize());
-
-    REQUIRE_NOTHROW(TriangleSelector::has_facets(data, EnforcerBlockerType::Extruder3));
-
-    TriangleSelector::TriangleSplittingData recomputed = data;
-    recomputed.reset_used_states();
-    REQUIRE_FALSE(recomputed.update_used_states(0));
-    REQUIRE(std::none_of(recomputed.used_states.begin(), recomputed.used_states.end(), [](bool used) { return used; }));
-}

tests/libslic3r/test_utils.cpp

@@ -152,3 +152,68 @@ TEST_CASE("resolve_cli_input_path leaves inputs that must not be completed uncha
         REQUIRE(resolve_cli_input_path("").empty());
     }
 }
+
+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";
+
+    SECTION("a file inside the root") {
+        REQUIRE(is_absolute_path_within_root(inside, root));
+    }
+    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));
+    }
+#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));
+    }
+#endif
+}
+
+TEST_CASE("is_safe_to_open_file_name accepts plain documents, images and models", "[utils]") {
+    const std::string safe = GENERATE(as<std::string>{},
+        "Manual.pdf", "BOM.xlsx", "BOM.csv", "guide.docx", "notes.txt", "README.md", "photo.JPG", "render.png",
+        "assembly.step", "part.stl", "project.3mf", "drawing.dxf", "build.mp4", "setup.exe.pdf", ".pdf");
+    INFO(safe);
+    CHECK(is_safe_to_open_file_name(safe));
+}
+
+TEST_CASE("is_safe_to_open_file_name rejects programs and anything it does not know", "[utils]") {
+    const std::string unsafe = GENERATE(as<std::string>{},
+        "setup.exe", "SETUP.EXE", "Manual.pdf.exe", "run.bat", "shortcut.lnk", "site.url", "script.ps1", "help.chm",
+        "tool.jar", "script.py", "Install.command", "install.sh", "launcher.desktop", "Printer.AppImage",
+        // Documents that can carry macros or scripts.
+        "BOM.xls", "BOM.xlsm", "guide.doc", "guide.docm", "sheet.ods", "page.html", "logo.svg", "bundle.zip",
+        // No extension, an unknown one, or a name the desktop would read differently.
+        "readme", "pdf", "data.xyz", "", "...", "Manual.pdf.", "Manual.pdf ", "setup.exe:note.txt", "dir.pdf/readme");
+    INFO(unsafe);
+    CHECK_FALSE(is_safe_to_open_file_name(unsafe));
+}