From b09667ea8910cf89a2c46f15b5f5788d4c7a4435 Mon Sep 17 00:00:00 2001
From: Joseph Lenox <lenox.joseph@gmail.com>
Date: Sun, 14 Mar 2021 19:32:14 -0500
Subject: [PATCH] Don't add any facets to the read-in list if it calls out a
 vertex that 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
---
 src/CMakeLists.txt                          |  1 +
 src/test/inputs/test_amf/5061-malicious.xml | 20 +++++++++++++++
 src/test/inputs/test_amf/read-amf.xml       | 19 ++++++++++++++
 src/test/libslic3r/test_amf.cpp             | 28 +++++++++++++++++++++
 xs/src/libslic3r/IO/AMF.cpp                 | 13 +++++++---
 5 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 src/test/inputs/test_amf/5061-malicious.xml
 create mode 100644 src/test/inputs/test_amf/read-amf.xml
 create mode 100644 src/test/libslic3r/test_amf.cpp

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 95f21da34..7c78c782d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -332,6 +332,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
new file mode 100644
index 000000000..9cde996f6
--- /dev/null
+++ 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
new file mode 100644
index 000000000..f366b2be4
--- /dev/null
+++ 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
new file mode 100644
index 000000000..dcd73946b
--- /dev/null
+++ 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
index e6eecfde4..a7c3cca7d 100644
--- 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();