Don't add any facets to the read-in list if it calls out a vertex tha…

…t is more than we have read in. (assumes that we have the vertex list before the volumes, which should be guaranteed in a normal file). Fixes #5061
slic3r · Mar 15, 2021 · cba887a · cba887a
1 parent 72bb3b3
commit cba887a
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 4 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -323,6 +323,7 @@ set(SLIC3R_TEST_SOURCES
     ${TESTDIR}/libslic3r/test_trianglemesh.cpp
     ${TESTDIR}/libslic3r/test_extrusion_entity.cpp
     ${TESTDIR}/libslic3r/test_3mf.cpp
+    ${TESTDIR}/libslic3r/test_amf.cpp
 )
 
 

diff --git a/src/test/inputs/test_amf/5061-malicious.xml b/src/test/inputs/test_amf/5061-malicious.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="utf-8"?>
+<amf unit="inch" version="1.1">
+    <metadata type="name">Split Pyramid</metadata>
+    <metadata tyze="author">John Smith</metadata>
+    <object id="1">
+        <mesh>
+            <vertices>
+                <vertex><coordinates><x>2</x><y>0</y><z>0</z></coordinates></vertex>
+                <vertex><coordinates><x>2.5</x><y>0.5</y><z>0</z></coordinates></vertex>
+            </vertices>
+            <volume materialid="2">
+                <metadata type="name">Hard side</metadata>
+                <triangle><v1>2</v1><v2>1</v2><v3>2</v3></triangle>
+                <triangle><v1>0</v1><v2>0</v2><v3>2</v3></triangle>
+            </volume>
+        </mesh>
+    </object>
+    <material id="1">
+    </material>
+</amf>
diff --git a/src/test/inputs/test_amf/read-amf.xml b/src/test/inputs/test_amf/read-amf.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="utf-8"?>
+<amf unit="inch" version="1.1">
+    <metadata type="name">Split Pyramid</metadata>
+    <metadata tyze="author">John Smith</metadata>
+    <object id="1">
+        <mesh>
+            <vertices>
+                <vertex><coordinates><x>2</x><y>0</y><z>0</z></coordinates></vertex>
+                <vertex><coordinates><x>2.5</x><y>0.5</y><z>0</z></coordinates></vertex>
+                <vertex><coordinates><x>3.5</x><y>2.5</y><z>0</z></coordinates></vertex>
+            </vertices>
+            <volume materialid="2">
+                <metadata type="name">Hard side</metadata>
+                <triangle><v1>2</v1><v2>1</v2><v3>2</v3></triangle>
+                <triangle><v1>0</v1><v2>0</v2><v3>2</v3></triangle>
+            </volume>
+        </mesh>
+    </object>
+</amf>
diff --git a/src/test/libslic3r/test_amf.cpp b/src/test/libslic3r/test_amf.cpp
@@ -0,0 +1,28 @@
+#include <catch.hpp>
+#include <test_options.hpp>
+#include "Model.hpp"
+#include "IO.hpp"
+
+
+using namespace Slic3r;
+
+SCENARIO("Reading AMF file") {
+    GIVEN("badly formed AMF file (missing vertices)") {
+        auto model {new Slic3r::Model()};
+        WHEN("AMF model is read") {
+            auto ret = Slic3r::IO::AMF::read(testfile("test_amf/5061-malicious.xml"),model);
+            THEN("read should return True") {
+                REQUIRE(ret);
+            }
+        }
+    }
+    GIVEN("Ok formed AMF file") {
+        auto model {new Slic3r::Model()};
+        WHEN("AMF model is read") {
+            auto ret = Slic3r::IO::AMF::read(testfile("test_amf/read-amf.xml"),model);
+            THEN("read should return True") {
+                REQUIRE(ret);
+            }
+        }
+    }
+}
diff --git a/xs/src/libslic3r/IO/AMF.cpp b/xs/src/libslic3r/IO/AMF.cpp
@@ -344,9 +344,13 @@ void AMFParserContext::endElement(const char *name)
     // Faces of the current volume:
     case NODE_TYPE_TRIANGLE:
         assert(m_object && m_volume);
-        m_volume_facets.push_back(atoi(m_value[0].c_str()));
-        m_volume_facets.push_back(atoi(m_value[1].c_str()));
-        m_volume_facets.push_back(atoi(m_value[2].c_str()));
+        if (strtoul(m_value[0].c_str(), nullptr, 10) < m_object_vertices.size() &&
+            strtoul(m_value[1].c_str(), nullptr, 10) < m_object_vertices.size() &&
+            strtoul(m_value[2].c_str(), nullptr, 10) < m_object_vertices.size()) {
+            m_volume_facets.push_back(atoi(m_value[0].c_str()));
+            m_volume_facets.push_back(atoi(m_value[1].c_str()));
+            m_volume_facets.push_back(atoi(m_value[2].c_str()));
+        }
         m_value[0].clear();
         m_value[1].clear();
         m_value[2].clear();
@@ -363,8 +367,9 @@ void AMFParserContext::endElement(const char *name)
         stl_allocate(&stl);
         for (size_t i = 0; i < m_volume_facets.size();) {
             stl_facet &facet = stl.facet_start[i/3];
-            for (unsigned int v = 0; v < 3; ++ v)
+            for (unsigned int v = 0; v < 3; ++ v) {
                 memcpy(&facet.vertex[v].x, &m_object_vertices[m_volume_facets[i ++] * 3], 3 * sizeof(float));
+            }
         }
         stl_get_size(&stl);
         m_volume->mesh.repair();