From 2045e22463324485a3332fa83e6a6a81ad094caa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Fri, 13 Mar 2020 20:27:00 +0100
Subject: [PATCH] python: properly check also level bounds in importers.

---
 src/python/magnum/test/test_trade.py | 10 ++++++++++
 src/python/magnum/trade.cpp          | 15 ++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/python/magnum/test/test_trade.py b/src/python/magnum/test/test_trade.py
index 496019f..71033c0 100644
--- a/src/python/magnum/test/test_trade.py
+++ b/src/python/magnum/test/test_trade.py
@@ -192,6 +192,8 @@ class Importer(unittest.TestCase):
 
         with self.assertRaises(IndexError):
             importer.image1d(0)
+        with self.assertRaises(IndexError):
+            importer.image2d(0, 1)
         with self.assertRaises(IndexError):
             importer.image2d(1)
         with self.assertRaises(IndexError):
@@ -217,6 +219,14 @@ class Importer(unittest.TestCase):
         mesh = importer.mesh(0)
         self.assertEqual(mesh.primitive, MeshPrimitive.TRIANGLES)
 
+    def test_mesh_index_oob(self):
+        # importer refcounting tested in image2d
+        importer = trade.ImporterManager().load_and_instantiate('TinyGltfImporter')
+        importer.open_file(os.path.join(os.path.dirname(__file__), 'mesh.glb'))
+
+        with self.assertRaises(IndexError):
+            importer.mesh(0, 1)
+
     def test_image2d(self):
         manager = trade.ImporterManager()
         manager_refcount = sys.getrefcount(manager)
diff --git a/src/python/magnum/trade.cpp b/src/python/magnum/trade.cpp
index 581bfe0..6274c62 100644
--- a/src/python/magnum/trade.cpp
+++ b/src/python/magnum/trade.cpp
@@ -190,7 +190,7 @@ template<class R, Containers::Optional<R>(Trade::AbstractImporter::*f)(UnsignedI
     return *std::move(out);
 }
 
-template<class R, Containers::Optional<R>(Trade::AbstractImporter::*f)(UnsignedInt, UnsignedInt), UnsignedInt(Trade::AbstractImporter::*bounds)() const> R checkOpenedBoundsResult(Trade::AbstractImporter& self, UnsignedInt id, UnsignedInt level) {
+template<class R, Containers::Optional<R>(Trade::AbstractImporter::*f)(UnsignedInt, UnsignedInt), UnsignedInt(Trade::AbstractImporter::*bounds)() const, UnsignedInt(Trade::AbstractImporter::*levelBounds)(UnsignedInt)> R checkOpenedBoundsResult(Trade::AbstractImporter& self, UnsignedInt id, UnsignedInt level) {
     if(!self.isOpened()) {
         PyErr_SetString(PyExc_RuntimeError, "no file opened");
         throw py::error_already_set{};
@@ -201,6 +201,11 @@ template<class R, Containers::Optional<R>(Trade::AbstractImporter::*f)(UnsignedI
         throw py::error_already_set{};
     }
 
+    if(level >= (self.*levelBounds)(id)) {
+        PyErr_SetNone(PyExc_IndexError);
+        throw py::error_already_set{};
+    }
+
     /** @todo log redirection -- but we'd need assertions to not be part of
         that so when it dies, the user can still see why */
     Containers::Optional<R> out = (self.*f)(id, level);
@@ -266,7 +271,7 @@ void trade(py::module& m) {
         .def("mesh_level_count", checkOpenedBounds<UnsignedInt, &Trade::AbstractImporter::meshLevelCount, &Trade::AbstractImporter::meshCount>, "Mesh level count", py::arg("id"))
         .def("mesh_for_name", checkOpened<Int, const std::string&, &Trade::AbstractImporter::meshForName>, "Mesh ID for given name")
         .def("mesh_name", checkOpenedBounds<std::string, &Trade::AbstractImporter::meshName, &Trade::AbstractImporter::meshCount>, "Mesh name", py::arg("id"))
-        .def("mesh", checkOpenedBoundsResult<Trade::MeshData, &Trade::AbstractImporter::mesh, &Trade::AbstractImporter::meshCount>, "Mesh", py::arg("id"), py::arg("level") = 0)
+        .def("mesh", checkOpenedBoundsResult<Trade::MeshData, &Trade::AbstractImporter::mesh, &Trade::AbstractImporter::meshCount, &Trade::AbstractImporter::meshLevelCount>, "Mesh", py::arg("id"), py::arg("level") = 0)
         /** @todo mesh_attribute_for_name / mesh_attribute_name */
 
         .def_property_readonly("image1d_count", checkOpened<UnsignedInt, &Trade::AbstractImporter::image1DCount>, "One-dimensional image count")
@@ -281,9 +286,9 @@ void trade(py::module& m) {
         .def("image1d_name", checkOpenedBounds<std::string, &Trade::AbstractImporter::image1DName, &Trade::AbstractImporter::image1DCount>, "One-dimensional image name", py::arg("id"))
         .def("image2d_name", checkOpenedBounds<std::string, &Trade::AbstractImporter::image2DName, &Trade::AbstractImporter::image2DCount>, "Two-dimensional image name", py::arg("id"))
         .def("image3d_name", checkOpenedBounds<std::string, &Trade::AbstractImporter::image3DName, &Trade::AbstractImporter::image3DCount>, "Three-dimensional image name", py::arg("id"))
-        .def("image1d", checkOpenedBoundsResult<Trade::ImageData1D, &Trade::AbstractImporter::image1D, &Trade::AbstractImporter::image1DCount>, "One-dimensional image", py::arg("id"), py::arg("level") = 0)
-        .def("image2d", checkOpenedBoundsResult<Trade::ImageData2D, &Trade::AbstractImporter::image2D, &Trade::AbstractImporter::image2DCount>, "Two-dimensional image", py::arg("id"), py::arg("level") = 0)
-        .def("image3d", checkOpenedBoundsResult<Trade::ImageData3D, &Trade::AbstractImporter::image3D, &Trade::AbstractImporter::image3DCount>, "Three-dimensional image", py::arg("id"), py::arg("level") = 0);
+        .def("image1d", checkOpenedBoundsResult<Trade::ImageData1D, &Trade::AbstractImporter::image1D, &Trade::AbstractImporter::image1DCount, &Trade::AbstractImporter::image1DLevelCount>, "One-dimensional image", py::arg("id"), py::arg("level") = 0)
+        .def("image2d", checkOpenedBoundsResult<Trade::ImageData2D, &Trade::AbstractImporter::image2D, &Trade::AbstractImporter::image2DCount, &Trade::AbstractImporter::image2DLevelCount>, "Two-dimensional image", py::arg("id"), py::arg("level") = 0)
+        .def("image3d", checkOpenedBoundsResult<Trade::ImageData3D, &Trade::AbstractImporter::image3D, &Trade::AbstractImporter::image3DCount, &Trade::AbstractImporter::image3DLevelCount>, "Three-dimensional image", py::arg("id"), py::arg("level") = 0);
 
     py::class_<PluginManager::Manager<Trade::AbstractImporter>, PluginManager::AbstractManager> importerManager{m, "ImporterManager", "Plugin manager for importer plugins"};
     corrade::manager(importerManager);