StatisticalOutlierRemoval: fix potential container overflow read

[steple]

nearestKSearch might reduce the size of nn_dists.

[mvieth]

@steple Hi, the only reason I can think of why nearestKSearch might return fewer than searcher_k points, is because the point cloud contains fewer than search_k points. Was this the case for you when you experienced the container overflow? Do you know which search method was used? (was the point cloud organized or not?)

[steple]

I don't have answers to your questions readily available. I ran into this when upgrading compilers, and the new compiler catches this issue in the act (with sanitizers). I'll try to look into it. (I don't usually work in that part of the codebase.)

I went for what I thought is obvious fix, but alternatively the could could also assert and explode, I guess.

[mvieth]

Ah okay. Which compiler are you using, and which sanitizer? Could this have been a false positive? With static analyzers for example, I have seen several cases where the analyzer has found an issue, but on closer inspection it was a false positive.
I am fine with switching to nn_dists.size(), but I would like to fully understand the problem first. Who knows, there could be a problem hiding in one of the search methods, leading to the resize 😄
The division should be nn_dists.size() - 1 though (minus, not plus), and in the cpp file, it should stay distances[cp] (cp, not iii).

[steple]

clang 17 with asan.
I confirmed it with gdb.

The calling code looks somewhat like this

    pcl::PointCloud<pcl::PointXYZRGB>::Ptr cloud_filtered(new pcl::PointCloud<pcl::PointXYZRGB>);
    pcl::StatisticalOutlierRemoval<pcl::PointXYZRGB> sor;
    sor.setInputCloud(cloud);
    sor.setMeanK(...);
    sor.setStddevMulThresh(...);
    sor.filter(*cloud_filtered);

and I believe it hit this resize

template <typename PointT, typename Dist> int 
pcl::KdTreeFLANN<PointT, Dist>::nearestKSearch (const PointT &point, unsigned int k,
                                                Indices &k_indices,
                                                std::vector<float> &k_distances) const
{
  assert (point_representation_->isValid (point) && "Invalid (NaN, Inf) point coordinates given to nearestKSearch!");

  if (k > total_nr_points_)
    k = total_nr_points_;

  k_indices.resize (k);
  k_distances.resize (k);

[steple]

The division should be nn_dists.size() - 1 though (minus, not plus), and in the cpp file, it should stay distances[cp] (cp, not iii).

Done.

[mvieth]

Thanks for the additional information! Just two minor changes (to fix sign-compare problems), then I am happy with the pull request.

[mvieth]

Thank you!

[steple]

Thank you for making this quick and easy.