From 90b7c51e6f21cf43875a0b38a3201cac17b6bc0b Mon Sep 17 00:00:00 2001 From: Jean-Christophe Ruel Date: Fri, 9 Aug 2024 02:18:04 -0400 Subject: [PATCH 1/2] Add STL file format disclaimer and enhance deserialization validation - Added a disclaimer section to the README highlighting the limitations of the STL file format, including the lack of built-in validation mechanisms and the potential for buffer overflow attacks. - Improved deserialization function to include strict validation for file size and data integrity, particularly addressing the handling of truncated headers. --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index e4cd550..d0c61b0 100644 --- a/README.md +++ b/README.md @@ -209,3 +209,17 @@ ctest . # Requirements C++11 or higher. + + +# DISCLAIMER: STL File Format # + + +The STL file format, while widely used for 3D modeling and printing, was designed to be simple and easy to parse. However, this simplicity comes with some significant limitations: + +- Lack of Built-in Validation Mechanisms: The STL format does not include built-in mechanisms such as checksums, hashes, or any form of file validation. This makes it challenging to detect certain types of file corruption, such as a truncated header or malformed data. As a result, errors in file transmission, storage, or manipulation might go undetected. + +- Vulnerability to Corruption: Due to the lack of validation features, STL files can be easily corrupted. For example, if the file is truncated or contains invalid data, these issues may not be detected until the file is parsed or processed, potentially leading to crashes or undefined behavior in applications that use the file. + +- Potential for Buffer Overflow Attacks: The lack of built-in validation and the absence of bounds checking in the STL format can make it susceptible to buffer overflow attacks. Care should be taken when handling STL files, especially those from untrusted sources, to ensure they are properly validated before being used. + +These limitations are inherent to the STL format and should be considered when working with or implementing software that processes STL files. Developers are encouraged to implement additional validation and error-handling mechanisms in their applications to mitigate these risks. \ No newline at end of file From b95a894410ff20c390e749ac5d561320daa08df8 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Ruel Date: Fri, 9 Aug 2024 02:18:47 -0400 Subject: [PATCH 2/2] feat: Add validation for truncated headers in STL deserialization - Implemented strict validation in `deserializeBinaryStl` to ensure the file is large enough for a valid STL header (80 bytes) and triangle count (4 bytes). - Added test cases for handling truncated headers, verifying that deserialization throws an exception when the header is incomplete. - Improved error handling to check for file corruption or incompleteness during deserialization. --- modules/core/include/openstl/core/stl.h | 63 +++++-- tests/core/src/deserialize.test.cpp | 102 ++++++++++++ tests/core/src/serialize.test.cpp | 27 +-- tests/utils/include/openstl/tests/testutils.h | 154 +++++++++++++++++- 4 files changed, 311 insertions(+), 35 deletions(-) diff --git a/modules/core/include/openstl/core/stl.h b/modules/core/include/openstl/core/stl.h index d69e574..8af14fc 100644 --- a/modules/core/include/openstl/core/stl.h +++ b/modules/core/include/openstl/core/stl.h @@ -89,17 +89,19 @@ namespace openstl void serializeBinaryStl(const Container& triangles, Stream& stream) { // Write header (80 bytes for comments) char header[80] = "STL Exported by OpenSTL [https://github.com/Innoptech/OpenSTL]"; - stream.write(header, 80); + stream.write(header, sizeof(header)); // Write triangle count (4 bytes) auto triangleCount = static_cast(triangles.size()); - stream.write((const char*)&triangleCount, sizeof(triangleCount)); + stream.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); // Write triangles - for (const auto& tri : triangles) - stream.write((const char*)&tri, sizeof(Triangle)); + for (const auto& tri : triangles) { + stream.write(reinterpret_cast(&tri), sizeof(Triangle)); + } } + /** * @brief Serialize a vector of triangles in the specified STL format and write to a stream. * @@ -171,7 +173,6 @@ namespace openstl triangles.push_back(tri); } } - return triangles; } @@ -183,16 +184,56 @@ namespace openstl * @return A vector of triangles representing the geometry from the binary STL file. */ template - inline std::vector deserializeBinaryStl(Stream& stream) - { - // Read header - stream.ignore(80); // Ignore the header + std::vector deserializeBinaryStl(Stream& stream) { + // Get the current position and determine the file size + auto start_pos = stream.tellg(); + stream.seekg(0, std::ios::end); + auto end_pos = stream.tellg(); + stream.seekg(start_pos); + + // Ensure the file is large enough for the header and triangle count + if (end_pos - start_pos < 84) { + throw std::runtime_error("File is too small to be a valid STL file."); + } + + // Explicitly read the header (80 bytes) + char header[80]; + stream.read(header, sizeof(header)); + + if (stream.gcount() != sizeof(header)) { + throw std::runtime_error("Failed to read the full header. Possible corruption or incomplete file."); + } + + // Read and validate triangle count (4 bytes) uint32_t triangle_qty; - stream.read((char*)&triangle_qty, sizeof(triangle_qty)); + stream.read(reinterpret_cast(&triangle_qty), sizeof(triangle_qty)); + + if (stream.gcount() != sizeof(triangle_qty) || stream.fail() || stream.eof()) { + throw std::runtime_error("Failed to read the triangle count. Possible corruption or incomplete file."); + } + + // Validate triangle count + const uint32_t MAX_TRIANGLES = 1000000; + if (triangle_qty > MAX_TRIANGLES) { + throw std::runtime_error("Triangle count exceeds the maximum allowable value."); + } + + // Calculate the expected size of the triangle data + std::size_t expected_data_size = sizeof(Triangle) * triangle_qty; + + // Ensure the stream has enough data left + if (end_pos - stream.tellg() < static_cast(expected_data_size)) { + throw std::runtime_error("Not enough data in stream for the expected triangle count."); + } // Read triangles std::vector triangles(triangle_qty); - stream.read((char*)triangles.data(), sizeof(Triangle)*triangle_qty); + stream.read(reinterpret_cast(triangles.data()), expected_data_size); + + if (stream.gcount() != expected_data_size || stream.fail() || stream.eof()) { + throw std::runtime_error("Failed to read the expected number of triangles. Possible corruption or incomplete file."); + } + return triangles; } diff --git a/tests/core/src/deserialize.test.cpp b/tests/core/src/deserialize.test.cpp index 88f6954..06d42c5 100644 --- a/tests/core/src/deserialize.test.cpp +++ b/tests/core/src/deserialize.test.cpp @@ -117,5 +117,107 @@ TEST_CASE("Deserialize Binary STL", "[openstl]") { } } +TEST_CASE("Binary STL Serialization/Deserialization Security and Integrity Tests", + "[openstl][security][stl][serialization][deserialization][security]") +{ + SECTION("Incomplete triangle data - incomplete_triangle_data.stl") { + const std::vector &triangles = testutils::createTestTriangle(); + const std::string filename{"incomplete_triangle_data.stl"}; + testutils::createIncompleteTriangleData(triangles, filename); + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Test deserialization with corrupted header (invalid characters)") { + const std::vector& triangles = testutils::createTestTriangle(); + const std::string filename = "corrupted_header.stl"; + testutils::createCorruptedHeaderInvalidChars(triangles, filename); // Generate the file with invalid characters in the header + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + + std::vector deserialized_triangles; + CHECK_NOTHROW(deserialized_triangles = deserializeBinaryStl(file)); + REQUIRE(testutils::checkTrianglesEqual(deserialized_triangles, triangles)); + } + SECTION("Test deserialization with corrupted header (excess data after header)") { + const std::vector &triangles = testutils::createTestTriangle(); + const std::string filename{"excess_data_after_header.stl"}; + testutils::createCorruptedHeaderExcessData(triangles, + filename); // Generate the file with excess data after the header + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Test deserialization with excessive triangle count") { + const std::vector &triangles = testutils::createTestTriangle(); + const std::string filename{"excessive_triangle_count.stl"}; + testutils::createExcessiveTriangleCount(triangles,filename); // Generate the file with an excessive triangle count + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Test deserialization with the maximum number of triangles") { + const uint32_t MAX_TRIANGLES = 1000000; + const std::string filename = "max_triangles.stl"; + + // Create a file with exactly MAX_TRIANGLES triangles + std::vector triangles(MAX_TRIANGLES); + testutils::createStlWithTriangles(triangles, filename); + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + + // Test that deserialization works correctly for MAX_TRIANGLES + std::vector deserialized_triangles; + CHECK_NOTHROW(deserialized_triangles = deserializeBinaryStl(file)); + REQUIRE(deserialized_triangles.size() == MAX_TRIANGLES); + } + SECTION("Test deserialization exceeding the maximum number of triangles") { + const uint32_t EXCEEDING_TRIANGLES = 1'000'001; + const std::string filename = "exceeding_triangles.stl"; + + // Create a file with more than MAX_TRIANGLES triangles + std::vector triangles(EXCEEDING_TRIANGLES); + testutils::createStlWithTriangles(triangles, filename); + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + + // Test that deserialization throws an exception for exceeding MAX_TRIANGLES + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Test deserialization with an empty file") { + const std::string filename{"empty_triangles.stl"}; + testutils::createEmptyStlFile(filename); // Generate an empty file + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Buffer overflow on triangle count - buffer_overflow_triangle_count.stl") { + std::string filename = "buffer_overflow_triangle_count.stl"; + testutils::createBufferOverflowOnTriangleCount(filename); + + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + CHECK_THROWS_AS(deserializeBinaryStl(file), std::runtime_error); + } + SECTION("Test deserialization with corrupted header (invalid characters) - corrupted_header_invalid_chars.stl") { + const std::vector& triangles = testutils::createTestTriangle(); + const std::string filename = "corrupted_header_invalid_chars.stl"; + testutils::createCorruptedHeaderInvalidChars(triangles, filename); // Generate the file with invalid characters in the header + std::ifstream file(filename, std::ios::binary); + REQUIRE(file.is_open()); + + // Deserialize the STL file, ignoring the header content + auto deserialized_triangles = deserializeBinaryStl(file); + // Check that the deserialized triangles match the expected count and data + testutils::checkTrianglesEqual(deserialized_triangles, triangles); + } +} diff --git a/tests/core/src/serialize.test.cpp b/tests/core/src/serialize.test.cpp index 859074e..db01d99 100644 --- a/tests/core/src/serialize.test.cpp +++ b/tests/core/src/serialize.test.cpp @@ -1,29 +1,10 @@ #include +#include "openstl/tests/testutils.h" #include "openstl/core/stl.h" using namespace openstl; -// Custom equality operator for Vertex struct -bool operator!=(const Vec3& rhs, const Vec3& lhs) { - return std::tie(rhs.x, rhs.y, rhs.z) != std::tie(lhs.x, lhs.y, lhs.z); -} - -// Utility function to compare two vectors of triangles -bool compareTriangles(const std::vector& a, const std::vector& b, bool omit_attribute=false) { - if (a.size() != b.size()) return false; - for (size_t i = 0; i < a.size(); ++i) { - if (a[i].normal != b[i].normal || - a[i].v0 != b[i].v0 || - a[i].v1 != b[i].v1 || - a[i].v2 != b[i].v2 || - ((a[i].attribute_byte_count != b[i].attribute_byte_count) & !omit_attribute)) { - return false; - } - } - return true; -} - TEST_CASE("Serialize STL triangles", "[openstl]") { // Generate some sample triangles std::vector originalTriangles{ @@ -51,7 +32,7 @@ TEST_CASE("Serialize STL triangles", "[openstl]") { auto deserializedTriangles = deserializeBinaryStl(inFile); // Validate deserialized triangles against original triangles - REQUIRE(compareTriangles(deserializedTriangles, originalTriangles)); + REQUIRE(testutils::checkTrianglesEqual(deserializedTriangles, originalTriangles)); } SECTION("Binary Format stringstream") { @@ -65,7 +46,7 @@ TEST_CASE("Serialize STL triangles", "[openstl]") { auto deserializedTriangles = deserializeBinaryStl(ss); // Validate deserialized triangles against original triangles - REQUIRE(compareTriangles(deserializedTriangles, originalTriangles)); + REQUIRE(testutils::checkTrianglesEqual(deserializedTriangles, originalTriangles)); } SECTION("ASCII Format") { @@ -88,6 +69,6 @@ TEST_CASE("Serialize STL triangles", "[openstl]") { auto deserializedTriangles = deserializeAsciiStl(inFile); // Validate deserialized triangles against original triangles - REQUIRE(compareTriangles(deserializedTriangles, originalTriangles, true)); + REQUIRE(testutils::checkTrianglesEqual(deserializedTriangles, originalTriangles, true)); } } \ No newline at end of file diff --git a/tests/utils/include/openstl/tests/testutils.h b/tests/utils/include/openstl/tests/testutils.h index ce839a9..8fef781 100644 --- a/tests/utils/include/openstl/tests/testutils.h +++ b/tests/utils/include/openstl/tests/testutils.h @@ -1,12 +1,13 @@ #ifndef OPENSTL_TESTS_TESTUTILS_H #define OPENSTL_TESTS_TESTUTILS_H #include "assets/assets_path.h" +#include "openstl/core/stl.h" #include namespace openstl { namespace testutils { enum class TESTOBJECT { - KEY, BALL, WASHER + KEY, BALL, WASHER, COMPROMISED_TRIANGLE_COUNT, EMPTY_FILE }; inline std::string getTestObjectPath(TESTOBJECT obj) { @@ -24,6 +25,157 @@ namespace openstl { } return OPENSTL_TEST_ASSETSDIR + basename; } + + inline std::vector createTestTriangle() { + Triangle triangle; + triangle.normal = {0.1f, 0.2f, 1.0f}; + triangle.v0 = {0.0f, 0.0f, 0.0f}; + triangle.v1 = {1.0f, 0.0f, 0.0f}; + triangle.v2 = {0.0f, 1.0f, 0.0f}; + triangle.attribute_byte_count = 0; + return {triangle}; + } + + inline void createIncompleteTriangleData(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Write header (80 bytes for comments) + char header[80] = "STL Exported by OpenSTL [https://github.com/Innoptech/OpenSTL]"; + file.write(header, sizeof(header)); + + // Write triangle count (4 bytes) + auto triangleCount = static_cast(triangles.size()); + file.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); + + // Write only half of the triangles to simulate incomplete data + for (size_t i = 0; i < triangles.size() / 2; ++i) { + file.write(reinterpret_cast(&triangles[i]), sizeof(Triangle)); + } + file.close(); + } + + inline void createCorruptedHeaderTruncated(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Truncated header (less than 80 bytes) + char header[40] = "TruncatedHeader"; + file.write(header, sizeof(header)); // Writing only 40 bytes instead of 80 + + // Write triangle count and triangles normally + auto triangleCount = static_cast(triangles.size()); + file.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); + for (const auto& tri : triangles) { + file.write(reinterpret_cast(&tri), sizeof(Triangle)); + } + + file.close(); + } + + inline void createCorruptedHeaderExcessData(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Correct header followed by garbage data + char header[80] = "STL Exported by OpenSTL [https://github.com/Innoptech/OpenSTL]"; + file.write(header, sizeof(header)); + + // Write some garbage data to corrupt the file + char garbage[20] = "GARBAGE DATA"; + file.write(garbage, sizeof(garbage)); + + // Write triangle count and triangles normally + auto triangleCount = static_cast(triangles.size()); + file.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); + for (const auto& tri : triangles) { + file.write(reinterpret_cast(&tri), sizeof(Triangle)); + } + + file.close(); + } + + inline void createExcessiveTriangleCount(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Write header (80 bytes for comments) + char header[80] = "STL Exported by OpenSTL [https://github.com/Innoptech/OpenSTL]"; + file.write(header, sizeof(header)); + + // Write an excessive triangle count (much larger than actual size) + uint32_t excessiveCount = std::numeric_limits::max(); // Adding 1000 to the actual count + file.write(reinterpret_cast(&excessiveCount), sizeof(excessiveCount)); + file.close(); + } + + inline void createCorruptedHeaderInvalidChars(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Corrupted header with random invalid characters + char header[80] = "CorruptedHeader12345!@#$%&*()"; + file.write(header, sizeof(header)); + + // Write triangle count and triangles normally + auto triangleCount = static_cast(triangles.size()); + file.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); + for (const auto& tri : triangles) { + file.write(reinterpret_cast(&tri), sizeof(Triangle)); + } + + file.close(); + } + + inline void createBufferOverflowOnTriangleCount(const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Write only the header (80 bytes) and close the file + char header[80] = "STL Exported by OpenSTL [https://github.com/Innoptech/OpenSTL]"; + file.write(header, sizeof(header)); + file.close(); + } + + inline void createStlWithTriangles(const std::vector& triangles, const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + + // Write a valid header + char header[80] = "STL Exported by Test"; + file.write(header, sizeof(header)); + + // Write the number of triangles + uint32_t triangleCount = static_cast(triangles.size()); + file.write(reinterpret_cast(&triangleCount), sizeof(triangleCount)); + + // Write the triangles + file.write(reinterpret_cast(triangles.data()), triangles.size() * sizeof(Triangle)); + + file.close(); + } + + + inline void createEmptyStlFile(const std::string& filename) { + std::ofstream file(filename, std::ios::binary); + // Simply create the file and close it without writing anything to it + file.close(); + } + + // Custom equality operator for Vertex struct + inline bool operator!=(const Vec3& rhs, const Vec3& lhs) { + return std::tie(rhs.x, rhs.y, rhs.z) != std::tie(lhs.x, lhs.y, lhs.z); + } + + // Utility function to compare two vectors of triangles + inline bool checkTrianglesEqual(const std::vector& a, const std::vector& b, bool omit_attribute=false) { + if (a.size() != b.size()) return false; + for (size_t i = 0; i < a.size(); ++i) { + if (a[i].normal != b[i].normal || + a[i].v0 != b[i].v0 || + a[i].v1 != b[i].v1 || + a[i].v2 != b[i].v2 || + ((a[i].attribute_byte_count != b[i].attribute_byte_count) & !omit_attribute)) { + return false; + } + } + return true; + } + + } //namespace testutils } //namespace openstl #endif //OPENSTL_TESTS_TESTUTILS_H