From fa280529582fba1f3280d79906ba5df3c987ca75 Mon Sep 17 00:00:00 2001 From: paxcut <53811119+paxcut@users.noreply.github.com> Date: Sun, 15 Sep 2024 06:19:41 -0700 Subject: [PATCH] fix: Avoid crashing ImHex when passing invalid data to the 3D visualizer (#1874) ### Problem description There are some obvious constrains in the various array data sizes that we can use to determine if their sizes and in some cases their values are correct. ### Implementation description To test their validity, the most important of the various arrays are the indices, so their definition and use has been simplified a great deal. In order to treat all variables in a similar manner the changes to the uv variable posted in an earlier PR were also added here. With this one, all the recently opened PR's combined should contain exactly the same changes and fixes as the older 3-d visualizer PR #1850 which I will close shortly after this one is opened. --------- Co-authored-by: Nik --- .../content/pl_visualizers/3d_model.cpp | 451 +++++++++--------- 1 file changed, 224 insertions(+), 227 deletions(-) diff --git a/plugins/visualizers/source/content/pl_visualizers/3d_model.cpp b/plugins/visualizers/source/content/pl_visualizers/3d_model.cpp index be5bad1fd..a0aca6de8 100644 --- a/plugins/visualizers/source/content/pl_visualizers/3d_model.cpp +++ b/plugins/visualizers/source/content/pl_visualizers/3d_model.cpp @@ -18,6 +18,7 @@ #include #include +#include namespace hex::plugin::visualizers { @@ -27,43 +28,39 @@ namespace hex::plugin::visualizers { U8, U16, U32, - Invalid, + Undefined, }; + template struct Vectors { std::vector vertices; std::vector normals; std::vector colors; - std::vector uv1; - std::vector indices8; - std::vector indices16; - std::vector indices32; + std::vector uv; + std::vector indices; }; + template struct LineVectors { std::vector vertices; std::vector colors; - std::vector indices8; - std::vector indices16; - std::vector indices32; + std::vector indices; }; + template struct Buffers { gl::Buffer vertices; gl::Buffer normals; gl::Buffer colors; - gl::Buffer uv1; - gl::Buffer indices8; - gl::Buffer indices16; - gl::Buffer indices32; + gl::Buffer uv; + gl::Buffer indices; }; + template struct LineBuffers { gl::Buffer vertices; gl::Buffer colors; - gl::Buffer indices8; - gl::Buffer indices16; - gl::Buffer indices32; + gl::Buffer indices; }; ImVec2 s_renderingWindowSize; @@ -84,6 +81,8 @@ namespace hex::plugin::visualizers { bool s_shouldUpdateLightSource = true; bool s_shouldUpdateTexture = false; + std::vector s_badIndices; + IndexType s_indexType; ImGuiExt::Texture s_modelTexture; @@ -97,6 +96,13 @@ namespace hex::plugin::visualizers { ImGuiExt::Texture s_texture; std::fs::path s_texturePath; + u32 s_vertexCount; + + const auto isIndexInRange = [](auto index) { + if (index >= s_vertexCount) + s_badIndices.push_back(index); + return index < s_vertexCount; + }; template void indicesForLines(std::vector &vertexIndices) { @@ -145,10 +151,18 @@ namespace hex::plugin::visualizers { if (minCamera[3] != 0) minCamera = minCamera * (1.0F / minCamera[3]); - float maxx = std::max(std::fabs(minCamera[0]), std::fabs(maxCamera[0])); - float maxy = std::max(std::fabs(minCamera[1]), std::fabs(maxCamera[1])); + float max_X = std::max(std::fabs(minCamera[0]), std::fabs(maxCamera[0])); + float max_Y = std::max(std::fabs(minCamera[1]), std::fabs(maxCamera[1])); - return std::max(maxx, maxy); + return std::max(max_X, max_Y); + } + + void setDefaultUVs(std::vector &uv, size_t size) { + uv.resize(size / 3 * 2); + for (u32 i = 0; i < uv.size(); i += 2) { + uv[i ] = 0.0F; + uv[i + 1] = 0.0F; + } } void setDefaultColors(std::vector &colors, size_t size, u32 color) { @@ -230,7 +244,8 @@ namespace hex::plugin::visualizers { } } - void loadVectors(Vectors &vectors, IndexType indexType) { + template + void loadVectors(Vectors &vectors, IndexType indexType) { s_max = getBoundingBox(vectors.vertices); if (s_drawTexture) @@ -238,62 +253,36 @@ namespace hex::plugin::visualizers { else if (vectors.colors.empty()) setDefaultColors(vectors.colors, vectors.vertices.size(), 0xFF337FFF); + if (vectors.uv.empty()) + setDefaultUVs(vectors.uv, vectors.vertices.size()); + if (vectors.normals.empty()) { vectors.normals.resize(vectors.vertices.size()); - if ((indexType == IndexType::U8 && vectors.indices8.empty()) || (indexType == IndexType::Invalid) || - (indexType == IndexType::U16 && vectors.indices16.empty()) || - (indexType == IndexType::U32 && vectors.indices32.empty())) { - + if (vectors.indices.empty() || (indexType == IndexType::Undefined)) { setNormals(vectors.vertices, vectors.normals); } else { std::vector indices; - if (indexType == IndexType::U16) { - indices.resize(vectors.indices16.size()); - for (u32 i = 0; i < vectors.indices16.size(); ++i) - indices[i] = vectors.indices16[i]; + indices.resize(vectors.indices.size()); + for (u32 i = 0; i < vectors.indices.size(); i += 1) + indices[i] = vectors.indices[i]; - } else if (indexType == IndexType::U8) { - indices.resize(vectors.indices8.size()); - for (u32 i = 0; i < vectors.indices8.size(); ++i) - indices[i] = vectors.indices8[i]; - - } else { - indices.resize(vectors.indices32.size()); - for (u32 i = 0; i < vectors.indices32.size(); ++i) - indices[i] = vectors.indices32[i]; - } setNormalsWithIndices(vectors.vertices, vectors.normals, indices); } } } - void loadLineVectors(LineVectors &lineVectors, IndexType indexType) { - const auto vertexCount = lineVectors.vertices.size(); + template + void loadLineVectors(LineVectors &lineVectors, IndexType indexType) { s_max = getBoundingBox(lineVectors.vertices); if (lineVectors.colors.empty()) setDefaultColors(lineVectors.colors, lineVectors.vertices.size(), 0xFF337FFF); - std::vector indices; - if (indexType == IndexType::U8) - indicesForLines(lineVectors.indices8); - else if (indexType == IndexType::U16) - indicesForLines(lineVectors.indices16); - else - indicesForLines(lineVectors.indices32); - - const auto isIndexInRange = [vertexCount](auto index) { return index >= vertexCount; }; - - if ( - !std::ranges::all_of(lineVectors.indices8, isIndexInRange) || - !std::ranges::all_of(lineVectors.indices16, isIndexInRange) || - !std::ranges::all_of(lineVectors.indices32, isIndexInRange) - ) { - throw std::logic_error("One or more indices point to out-of-range vertex"); - } + if (indexType != IndexType::Undefined) + indicesForLines(lineVectors.indices); } void processKeyEvent(ImGuiKey key, float &variable, float increment, float acceleration) { @@ -352,75 +341,104 @@ namespace hex::plugin::visualizers { rotation[2] = std::fmod(rotation[2], 2 * std::numbers::pi_v); } + bool validateVector(const std::vector &vector, u32 vertexCount, u32 divisor, const std::string &name,std::string &errorMessage) { + if (!vector.empty()) { + if (vector.size() % divisor != 0) { + errorMessage = name + " must be a multiple of " + std::to_string(divisor); + return false; + } + } else { + errorMessage = name + " can't be empty"; + return false; + } + auto vectorCount = vector.size()/divisor; + if (vectorCount != vertexCount) { + errorMessage = hex::format("Expected {} colors, got {}", vertexCount, vectorCount); + return false; + } + return true; + } - void bindBuffers(Buffers &buffers, const gl::VertexArray &vertexArray, Vectors vectors, IndexType indexType) { + template + void bindBuffers(Buffers &buffers, const gl::VertexArray &vertexArray, Vectors vectors, IndexType indexType) { buffers.vertices = {}; buffers.normals = {}; buffers.colors = {}; - buffers.uv1 = {}; - buffers.indices8 = {}; - buffers.indices16 = {}; - buffers.indices32 = {}; + buffers.uv = {}; + buffers.indices = {}; vertexArray.bind(); - buffers.vertices = gl::Buffer(gl::BufferType::Vertex, vectors.vertices); - buffers.colors = gl::Buffer(gl::BufferType::Vertex, vectors.colors); - buffers.normals = gl::Buffer(gl::BufferType::Vertex, vectors.normals); + u32 vertexCount = vectors.vertices.size() / 3; + std::string errorMessage; - if (indexType == IndexType::U8) - buffers.indices8 = gl::Buffer(gl::BufferType::Index, vectors.indices8); - else if (indexType == IndexType::U16) - buffers.indices16 = gl::Buffer(gl::BufferType::Index, vectors.indices16); + if (indexType != IndexType::Undefined && !vectors.indices.empty()) + buffers.indices = gl::Buffer(gl::BufferType::Index, vectors.indices); + + if (validateVector(vectors.vertices, vertexCount, 3, "Positions", errorMessage)) { + if ((indexType == IndexType::Undefined || vectors.indices.empty()) && vertexCount % 3 != 0) + throw std::runtime_error("Without indices vertices must be a multiple of 3"); + else + buffers.vertices = gl::Buffer(gl::BufferType::Vertex, vectors.vertices); + } else + throw std::runtime_error(errorMessage); + + if (validateVector(vectors.colors, vertexCount, 4, "Colors", errorMessage)) + buffers.colors = gl::Buffer(gl::BufferType::Vertex, vectors.colors); else - buffers.indices32 = gl::Buffer(gl::BufferType::Index, vectors.indices32); + throw std::runtime_error(errorMessage); + + if (validateVector(vectors.normals, vertexCount, 3, "Normals", errorMessage)) + buffers.normals = gl::Buffer(gl::BufferType::Vertex, vectors.normals); + else + throw std::runtime_error(errorMessage); + + if (validateVector(vectors.uv, vertexCount, 2, "UV coordinates", errorMessage)) + buffers.uv = gl::Buffer(gl::BufferType::Vertex, vectors.uv); + else + throw std::runtime_error(errorMessage); - if (!vectors.uv1.empty()) - buffers.uv1 = gl::Buffer(gl::BufferType::Vertex, vectors.uv1); vertexArray.addBuffer(0, buffers.vertices); vertexArray.addBuffer(1, buffers.colors, 4); vertexArray.addBuffer(2, buffers.normals); - - if (!vectors.uv1.empty()) - vertexArray.addBuffer(3, buffers.uv1, 2); + vertexArray.addBuffer(3, buffers.uv, 2); buffers.vertices.unbind(); buffers.colors.unbind(); buffers.normals.unbind(); + buffers.uv.unbind(); - if (!vectors.uv1.empty()) - buffers.uv1.unbind(); - - if (indexType == IndexType::U8) - buffers.indices8.unbind(); - - else if (indexType == IndexType::U16) - buffers.indices16.unbind(); - - else if (indexType == IndexType::U32) - buffers.indices32.unbind(); + if (indexType != IndexType::Undefined) + buffers.indices.unbind(); vertexArray.unbind(); } - void bindLineBuffers(LineBuffers &lineBuffers, const gl::VertexArray &vertexArray, const LineVectors &lineVectors, IndexType indexType) { + template + void bindLineBuffers(LineBuffers &lineBuffers, const gl::VertexArray &vertexArray, const LineVectors &lineVectors, IndexType indexType) { lineBuffers.vertices = {}; lineBuffers.colors = {}; - lineBuffers.indices8 = {}; - lineBuffers.indices16 = {}; - lineBuffers.indices32 = {}; - + lineBuffers.indices = {}; + u32 vertexCount = lineVectors.vertices.size() / 3; vertexArray.bind(); - lineBuffers.vertices = gl::Buffer(gl::BufferType::Vertex, lineVectors.vertices); - lineBuffers.colors = gl::Buffer(gl::BufferType::Vertex, lineVectors.colors); + std::string errorMessage; - if (indexType == IndexType::U8) - lineBuffers.indices8 = gl::Buffer(gl::BufferType::Index, lineVectors.indices8); - else if (indexType == IndexType::U16) - lineBuffers.indices16 = gl::Buffer(gl::BufferType::Index, lineVectors.indices16); + if (indexType != IndexType::Undefined) + lineBuffers.indices = gl::Buffer(gl::BufferType::Index, lineVectors.indices); + + if (validateVector(lineVectors.vertices, vertexCount, 3, "Positions", errorMessage)) { + if ((indexType == IndexType::Undefined || lineVectors.indices.empty()) && vertexCount % 3 != 0) + throw std::runtime_error("Without indices vertices must be a multiple of 3"); + else + lineBuffers.vertices = gl::Buffer(gl::BufferType::Vertex, lineVectors.vertices); + } else + throw std::runtime_error(errorMessage); + + if (validateVector(lineVectors.colors, vertexCount, 4, "Colors", errorMessage)) + lineBuffers.colors = gl::Buffer(gl::BufferType::Vertex, lineVectors.colors); else - lineBuffers.indices32 = gl::Buffer(gl::BufferType::Index, lineVectors.indices32); + throw std::runtime_error(errorMessage); vertexArray.addBuffer(0, lineBuffers.vertices); vertexArray.addBuffer(1, lineBuffers.colors, 4); @@ -428,12 +446,8 @@ namespace hex::plugin::visualizers { lineBuffers.vertices.unbind(); lineBuffers.colors.unbind(); - if (indexType == IndexType::U8) - lineBuffers.indices8.unbind(); - else if (indexType == IndexType::U16) - lineBuffers.indices16.unbind(); - else if (indexType == IndexType::U32) - lineBuffers.indices32.unbind(); + if (indexType != IndexType::Undefined) + lineBuffers.indices.unbind(); vertexArray.unbind(); @@ -587,7 +601,10 @@ namespace hex::plugin::visualizers { } - void draw3DVisualizer(pl::ptrn::Pattern &, bool shouldReset, std::span arguments) { + template + void processRendering(std::shared_ptr verticesPattern, std::shared_ptr indicesPattern, + std::shared_ptr normalsPattern, std::shared_ptr colorsPattern, + std::shared_ptr uvPattern) { static gl::LightSourceVectors sourceVectors(20); static gl::VertexArray sourceVertexArray = {}; static gl::LightSourceBuffers sourceBuffers(sourceVertexArray, sourceVectors); @@ -601,103 +618,74 @@ namespace hex::plugin::visualizers { static gl::AxesBuffers axesBuffers(axesVertexArray, axesVectors); static gl::VertexArray vertexArray = {}; - static Buffers buffers; - static LineBuffers lineBuffers; - - std::shared_ptr verticesPattern = arguments[0].toPattern(); - std::shared_ptr indicesPattern = arguments[1].toPattern(); - std::shared_ptr normalsPattern = nullptr; - std::shared_ptr colorsPattern = nullptr; - std::shared_ptr uv1Pattern = nullptr; - - std::string textureFile; - if (arguments.size() > 2) { - normalsPattern = arguments[2].toPattern(); - if (arguments.size() > 3) { - colorsPattern = arguments[3].toPattern(); - if (arguments.size() > 4) { - uv1Pattern = arguments[4].toPattern(); - if (arguments.size() > 5) - textureFile = arguments[5].toString(); - } - } - } - - if (shouldReset) - s_shouldReset = true; - - const auto fontSize = ImGui::GetFontSize(); - const auto framePad = ImGui::GetStyle().FramePadding; - float minSize = (fontSize * 8_scaled) + (framePad.x * 20_scaled); - minSize = minSize > 200_scaled ? minSize : 200_scaled; if (s_renderingWindowSize.x <= 0 || s_renderingWindowSize.y <= 0) - s_renderingWindowSize = { minSize, minSize }; - - if (!textureFile.empty()) { - s_texturePath = textureFile; - s_drawTexture = true; - } else { - s_drawTexture = false; - } - - s_renderingWindowSize.x = std::max(s_renderingWindowSize.x, minSize); - s_renderingWindowSize.y = std::max(s_renderingWindowSize.y, minSize); + s_renderingWindowSize = {350_scaled, 350_scaled}; gl::Matrix mvp(0); - - processInputEvents(s_rotation, s_translation, s_scaling, s_nearLimit, s_farLimit); + static Buffers buffers; + static LineBuffers lineBuffers; if (s_shouldReset) { s_shouldReset = false; - - auto *iterable = dynamic_cast(indicesPattern.get()); - if (iterable != nullptr && iterable->getEntryCount() > 0) { - const auto &content = iterable->getEntry(0); - if (content->getSize() == 1) { - s_indexType = IndexType::U8; - } else if (content->getSize() == 2) { - s_indexType = IndexType::U16; - } else if (content->getSize() == 4) { - s_indexType = IndexType::U32; - } else { - s_indexType = IndexType::Invalid; - } - } + s_shouldUpdateLightSource = true; if (s_drawMode == GL_TRIANGLES) { - Vectors vectors; + Vectors vectors; vectors.vertices = patternToArray(verticesPattern.get()); - if (s_indexType == IndexType::U16) - vectors.indices16 = patternToArray(indicesPattern.get()); - else if (s_indexType == IndexType::U32) - vectors.indices32 = patternToArray(indicesPattern.get()); - else if (s_indexType == IndexType::U8) - vectors.indices8 = patternToArray(indicesPattern.get()); + s_vertexCount = vectors.vertices.size() / 3; + if (s_indexType != IndexType::Undefined) { + vectors.indices = patternToArray(indicesPattern.get()); + s_badIndices.clear(); + auto indexCount = vectors.indices.size(); + if (indexCount < 3 || indexCount % 3 != 0) { + throw std::runtime_error("Index count must be a multiple of 3"); + } + auto booleans = std::views::transform(vectors.indices,isIndexInRange); + if (!std::accumulate(std::begin(booleans), std::end(booleans), true, std::logical_and<>())) { + std::string badIndicesStr = "Invalid indices: "; + for (auto badIndex : s_badIndices) + badIndicesStr += std::to_string(badIndex) + ", "; + badIndicesStr.pop_back(); + badIndicesStr.pop_back(); + badIndicesStr += hex::format(" for {} vertices",s_vertexCount); + throw std::runtime_error(badIndicesStr); + } + } if (colorsPattern != nullptr) vectors.colors = patternToArray(colorsPattern.get()); if (normalsPattern != nullptr) vectors.normals = patternToArray(normalsPattern.get()); - if (uv1Pattern != nullptr) - vectors.uv1 = patternToArray(uv1Pattern.get()); + if (uvPattern != nullptr) + vectors.uv = patternToArray(uvPattern.get()); loadVectors(vectors, s_indexType); bindBuffers(buffers, vertexArray, vectors, s_indexType); } else { - LineVectors lineVectors; + LineVectors lineVectors; lineVectors.vertices = patternToArray(verticesPattern.get()); - if (s_indexType == IndexType::U16) - lineVectors.indices16 = patternToArray(indicesPattern.get()); - - else if (s_indexType == IndexType::U32) - lineVectors.indices32 = patternToArray(indicesPattern.get()); - - else if (s_indexType == IndexType::U8) - lineVectors.indices8 = patternToArray(indicesPattern.get()); + s_vertexCount = lineVectors.vertices.size() / 3; + if (s_indexType != IndexType::Undefined) { + lineVectors.indices = patternToArray(indicesPattern.get()); + auto indexCount = lineVectors.indices.size(); + if (indexCount < 3 || indexCount % 3 != 0) { + throw std::runtime_error("Index count must be a multiple of 3"); + } + s_badIndices.clear(); + if (!std::ranges::all_of(lineVectors.indices,isIndexInRange)) { + std::string badIndicesStr = "Invalid indices: "; + for (auto badIndex : s_badIndices) + badIndicesStr += std::to_string(badIndex) + ", "; + badIndicesStr.pop_back(); + badIndicesStr.pop_back(); + badIndicesStr += hex::format(" for {} vertices",s_vertexCount); + throw std::runtime_error(badIndicesStr); + } + } if (colorsPattern != nullptr) lineVectors.colors = patternToArray(colorsPattern.get()); @@ -805,34 +793,13 @@ namespace hex::plugin::visualizers { if (s_drawTexture) glBindTexture(GL_TEXTURE_2D, s_modelTexture); - if (s_indexType == IndexType::U8) { + buffers.indices.bind(); + if (buffers.indices.getSize() == 0) + buffers.vertices.draw(s_drawMode); + else + buffers.indices.draw(s_drawMode); + buffers.indices.unbind(); - buffers.indices8.bind(); - if (buffers.indices8.getSize() == 0) - buffers.vertices.draw(s_drawMode); - else - buffers.indices8.draw(s_drawMode); - buffers.indices8.unbind(); - - } else if (s_indexType == IndexType::U16) { - - buffers.indices16.bind(); - if (buffers.indices16.getSize() == 0) - buffers.vertices.draw(s_drawMode); - else - buffers.indices16.draw(s_drawMode); - buffers.indices16.unbind(); - } else { - - buffers.indices32.bind(); - if (buffers.indices32.getSize() == 0) - buffers.vertices.draw(s_drawMode); - else - buffers.indices32.draw(s_drawMode); - buffers.indices32.unbind(); - - - } } else { static gl::Shader lineShader = gl::Shader( romfs::get("shaders/default/lineVertex.glsl").string(), @@ -844,29 +811,14 @@ namespace hex::plugin::visualizers { lineShader.setUniform("viewMatrix", view); lineShader.setUniform("projectionMatrix", projection); vertexArray.bind(); - if (s_indexType == IndexType::U8) { - lineBuffers.indices8.bind(); - if (lineBuffers.indices8.getSize() == 0) - lineBuffers.vertices.draw(s_drawMode); - else - lineBuffers.indices8.draw(s_drawMode); - lineBuffers.indices8.unbind(); - } else if (s_indexType == IndexType::U16) { - lineBuffers.indices16.bind(); - if (lineBuffers.indices16.getSize() == 0) - lineBuffers.vertices.draw(s_drawMode); - else - lineBuffers.indices16.draw(s_drawMode); - lineBuffers.indices16.unbind(); - } else { - lineBuffers.indices32.bind(); - if (lineBuffers.indices32.getSize() == 0) - lineBuffers.vertices.draw(s_drawMode); - else - lineBuffers.indices32.draw(s_drawMode); - lineBuffers.indices32.unbind(); - } + lineBuffers.indices.bind(); + if (lineBuffers.indices.getSize() == 0) + lineBuffers.vertices.draw(s_drawMode); + else + lineBuffers.indices.draw(s_drawMode); + lineBuffers.indices.unbind(); + } if (s_drawGrid || s_drawAxes) { @@ -926,4 +878,49 @@ namespace hex::plugin::visualizers { } } + void draw3DVisualizer(pl::ptrn::Pattern &, bool shouldReset, std::span arguments) { + + std::shared_ptr verticesPattern = arguments[0].toPattern(); + std::shared_ptr indicesPattern = arguments[1].toPattern(); + std::shared_ptr normalsPattern = nullptr; + std::shared_ptr colorsPattern = nullptr; + std::shared_ptr uvPattern = nullptr; + + std::string textureFile; + if (arguments.size() > 2) { + normalsPattern = arguments[2].toPattern(); + if (arguments.size() > 3) { + colorsPattern = arguments[3].toPattern(); + if (arguments.size() > 4) { + uvPattern = arguments[4].toPattern(); + if (arguments.size() > 5) + textureFile = arguments[5].toString(); + } + } + } + + s_texturePath = textureFile; + s_drawTexture = !textureFile.empty(); + if (shouldReset) + s_shouldReset = true; + processInputEvents(s_rotation, s_translation, s_scaling, s_nearLimit, s_farLimit); + + auto *iterable = dynamic_cast(indicesPattern.get()); + if (iterable != nullptr && iterable->getEntryCount() > 0) { + const auto &content = iterable->getEntry(0); + if (content->getSize() == 1) { + s_indexType = IndexType::U8; + processRendering(verticesPattern, indicesPattern, normalsPattern, colorsPattern, uvPattern); + } else if (content->getSize() == 2) { + s_indexType = IndexType::U16; + processRendering(verticesPattern, indicesPattern, normalsPattern, colorsPattern, uvPattern); + } else if (content->getSize() == 4) { + s_indexType = IndexType::U32; + processRendering(verticesPattern, indicesPattern, normalsPattern, colorsPattern, uvPattern); + } + } else { + s_indexType = IndexType::Undefined; + processRendering(verticesPattern, indicesPattern, normalsPattern, colorsPattern, uvPattern); + } + } } \ No newline at end of file