bound kd-tree node indices when loading a saved flann index

modules/features2d/test/test_nearestneighbors.cpp

@@ -328,6 +328,42 @@ TEST(Features2d_FLANN_Composite, regression) { CV_FlannCompositeIndexTest test;
 TEST(Features2d_FLANN_Auto, regression) { CV_FlannAutotunedIndexTest test; test.safe_run(); }
 TEST(Features2d_FLANN_Saved, regression) { CV_FlannSavedIndexTest test; test.safe_run(); }
 
+// A saved KD-tree index whose serialized leaf node carries a point index
+// outside the dataset must be rejected on load. Before the added validation
+// the malformed index loaded silently and the out-of-range index was
+// dereferenced during search (heap out-of-bounds access).
+TEST(Features2d_FLANN_KDTree, load_rejects_out_of_range_leaf_index)
+{
+    Mat features(1, 4, CV_32F);
+    features.at<float>(0, 0) = 1.f; features.at<float>(0, 1) = 2.f;
+    features.at<float>(0, 2) = 3.f; features.at<float>(0, 3) = 4.f;
+
+    const String filename = tempfile();
+    {
+        Index index(features, KDTreeIndexParams(1));
+        index.save(filename);
+    }
+
+    // Overwrite the single leaf node's divfeat (the first int of the last
+    // serialized Node record) with an index far outside the 1-row dataset.
+    {
+        FILE* f = fopen(filename.c_str(), "r+b");
+        ASSERT_TRUE(f != NULL);
+        ASSERT_EQ(0, fseek(f, 0, SEEK_END));
+        const long node_size = (long)(sizeof(int) + sizeof(float) + 2 * sizeof(void*));
+        const long size = ftell(f);
+        ASSERT_GT(size, node_size);
+        ASSERT_EQ(0, fseek(f, size - node_size, SEEK_SET));
+        const int out_of_range = 1 << 28;
+        ASSERT_EQ((size_t)1, fwrite(&out_of_range, sizeof(int), 1, f));
+        fclose(f);
+    }
+
+    Index loaded;
+    EXPECT_THROW(loaded.load(features, filename), cv::Exception);
+    remove(filename.c_str());
+}
+
 #endif
 
 }} // namespace

modules/flann/include/opencv2/flann/kdtree_index.h

@@ -266,6 +266,12 @@ private:
     {
         tree = pool_.allocate<Node>();
         load_value(stream, *tree);
+        if (tree->child1!=NULL || tree->child2!=NULL) {
+            // Internal node: divfeat is the split dimension and is used to index
+            // the query vector during search, so it must be a valid dimension.
+            if (tree->divfeat < 0 || (size_t)tree->divfeat >= veclen_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree index: split dimension is out of range");
+            }
             if (tree->child1!=NULL) {
                 load_tree(stream, tree->child1);
             }
@@ -273,6 +279,14 @@ private:
                 load_tree(stream, tree->child2);
             }
         }
+        else {
+            // Leaf node: divfeat is a dataset point index dereferenced during
+            // search, so it must fall inside the dataset.
+            if (tree->divfeat < 0 || (size_t)tree->divfeat >= size_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree index: leaf feature index is out of range");
+            }
+        }
+    }
 
 
     /**

modules/flann/include/opencv2/flann/kdtree_single_index.h

@@ -158,12 +158,33 @@ public:
     {
         load_value(stream, size_);
         load_value(stream, dim_);
+        // The dataset the index is attached to is fixed by the caller, so a
+        // saved index whose stored size/dim disagree with it is malformed.
+        if (size_ != dataset_.rows || dim_ != dataset_.cols) {
+            FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: saved dataset dimensions do not match");
+        }
         load_value(stream, root_bbox_);
+        if (root_bbox_.size() != dim_) {
+            FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: bounding box has wrong length");
+        }
         load_value(stream, reorder_);
         load_value(stream, leaf_max_size_);
         load_value(stream, vind_);
+        // vind_ holds one dataset point index per row and every entry is
+        // dereferenced during search.
+        if (vind_.size() != size_) {
+            FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: index permutation has wrong length");
+        }
+        for (size_t i = 0; i < vind_.size(); ++i) {
+            if (vind_[i] < 0 || (size_t)vind_[i] >= size_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: point index is out of range");
+            }
+        }
         if (reorder_) {
             load_value(stream, data_);
+            if (data_.rows != size_ || data_.cols != dim_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: reordered data has wrong shape");
+            }
         }
         else {
             data_ = dataset_;
@@ -303,6 +324,12 @@ private:
     {
         tree = pool_.allocate<Node>();
         load_value(stream, *tree);
+        if (tree->child1!=NULL || tree->child2!=NULL) {
+            // Internal node: divfeat is the split dimension, used to index the
+            // query vector and the per-dimension distance array during search.
+            if (tree->divfeat < 0 || (size_t)tree->divfeat >= dim_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: split dimension is out of range");
+            }
             if (tree->child1!=NULL) {
                 load_tree(stream, tree->child1);
             }
@@ -310,6 +337,14 @@ private:
                 load_tree(stream, tree->child2);
             }
         }
+        else {
+            // Leaf node: [left, right) is a range of point slots dereferenced
+            // during search, so it must stay inside the dataset.
+            if (tree->left < 0 || tree->right < tree->left || (size_t)tree->right > size_) {
+                FLANN_THROW(cv::Error::StsParseError, "FLANN kd-tree(single) index: leaf point range is out of range");
+            }
+        }
+    }
 
 
     void computeBoundingBox(BoundingBox& bbox)