fix: reject inconsistent point clouds; make ros_message_definitions.hpp includable twice

cloudini_lib/include/cloudini_lib/ros_message_definitions.hpp

@@ -16,10 +16,10 @@
 
 #pragma once
 
-const char* pointcloud_schema_name = "sensor_msgs/msg/PointCloud2";
-const char* compressed_schema_name = "point_cloud_interfaces/msg/CompressedPointCloud2";
+inline constexpr const char* pointcloud_schema_name = "sensor_msgs/msg/PointCloud2";
+inline constexpr const char* compressed_schema_name = "point_cloud_interfaces/msg/CompressedPointCloud2";
 
-const char* pointcloud_schema_data =
+inline constexpr const char* pointcloud_schema_data =
     R"(# This message holds a collection of N-dimensional points, which may
 # contain additional information such as normals, intensity, etc. The
 # point data is stored as a binary blob, its layout described by the
@@ -90,7 +90,7 @@ int32 sec
 uint32 nanosec
 )";
 
-const char* compressed_schema_data =
+inline constexpr const char* compressed_schema_data =
     R"(# SPDX-License-Identifier: BSD-3-Clause
 # SPDX-FileCopyrightText: Czech Technical University in Prague .. 2019, paplhjak
 

cloudini_lib/src/cloudini.cpp

@@ -428,6 +428,13 @@ EncodingInfo DecodeHeader(ConstBufferView& input) {
 }
 
 PointcloudEncoder::PointcloudEncoder(const EncodingInfo& info) : info_(info) {
+  // The field encoders read SizeOf(type) bytes at field.offset inside every point:
+  // a field that does not fit in point_step would read past the end of the cloud.
+  for (const auto& field : info_.fields) {
+    if (static_cast<uint64_t>(field.offset) + static_cast<uint64_t>(SizeOf(field.type)) > info_.point_step) {
+      throw std::runtime_error("PointcloudEncoder: field '" + field.name + "' does not fit in point_step");
+    }
+  }
   EncodeHeader(info_, header_);
 
   if (!detail::UsesV5Codec(info_)) {

cloudini_lib/src/ros_msg_utils.cpp

@@ -188,6 +188,14 @@ void convertPointCloud2ToCompressedCloud(
   if (encoding_info.point_step == 0) {
     throw std::runtime_error("convertPointCloud2ToCompressedCloud: point_step cannot be 0");
   }
+  // The header repeats width and height: if they disagree with the payload, the
+  // result cannot be decoded.
+  const uint64_t declared_size = uint64_t{pc_info.width} * pc_info.height * pc_info.point_step;
+  if (declared_size != pc_info.data.size()) {
+    throw std::runtime_error(
+        "convertPointCloud2ToCompressedCloud: width*height*point_step is " + std::to_string(declared_size) +
+        " but data has " + std::to_string(pc_info.data.size()) + " bytes");
+  }
   // Derive point count from actual data size rather than trusting metadata width*height,
   // which could be maliciously large and cause excessive allocation.
   const size_t points_count = pc_info.data.size() / encoding_info.point_step;

cloudini_lib/test/test_header.cpp

@@ -21,6 +21,7 @@
 #include <vector>
 
 #include "cloudini_lib/cloudini.hpp"
+#include "cloudini_lib/ros_message_definitions.hpp"  // second includer: see test_ros_msg.cpp
 
 namespace {
 

cloudini_lib/test/test_ros_msg.cpp

@@ -18,9 +18,11 @@
 
 #include <fstream>
 #include <iostream>
+#include <iterator>
 
 #include "cloudini_lib/cloudini.hpp"
 #include "cloudini_lib/encoding_utils.hpp"
+#include "cloudini_lib/ros_message_definitions.hpp"  // also included by test_header.cpp: must not break the link
 #include "cloudini_lib/ros_msg_utils.hpp"
 #include "data_path.hpp"
 
@@ -154,3 +156,47 @@ TEST(Cloudini, RosPointCloud2CopyRebindsOwnedDataView) {
   EXPECT_EQ(copied.data.data(), copied.owned_data.data());
   EXPECT_EQ(copied.data.size(), copied.owned_data.size());
 }
+
+namespace {
+std::vector<uint8_t> loadSampleDDSMessage() {
+  std::ifstream file(Cloudini::tests::DATA_PATH + "dds_message.bin", std::ios::binary);
+  EXPECT_TRUE(file.is_open());
+  return std::vector<uint8_t>((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
+}
+}  // namespace
+
+// A PointCloud2 can come from any DDS peer. Metadata that disagrees with the
+// payload must be rejected, not encoded: the field encoders read at
+// field.offset inside every point, and the header repeats width/height.
+TEST(Cloudini, InconsistentPointCloud2IsRejected) {
+  const auto dds_msg = loadSampleDDSMessage();
+  std::vector<uint8_t> output;
+
+  {
+    auto pc_info = cloudini_ros::getDeserializedPointCloudMessage(dds_msg);
+    pc_info.fields.back().offset = pc_info.point_step;  // field lies past the end of the point
+    const auto info = cloudini_ros::toEncodingInfo(pc_info);
+    EXPECT_THROW(cloudini_ros::convertPointCloud2ToCompressedCloud(pc_info, info, output), std::runtime_error);
+  }
+  {
+    auto pc_info = cloudini_ros::getDeserializedPointCloudMessage(dds_msg);
+    pc_info.width *= 2;  // header would promise twice the points that are encoded
+    const auto info = cloudini_ros::toEncodingInfo(pc_info);
+    EXPECT_THROW(cloudini_ros::convertPointCloud2ToCompressedCloud(pc_info, info, output), std::runtime_error);
+  }
+  {
+    auto pc_info = cloudini_ros::getDeserializedPointCloudMessage(dds_msg);
+    const auto info = cloudini_ros::toEncodingInfo(pc_info);
+    EXPECT_NO_THROW(cloudini_ros::convertPointCloud2ToCompressedCloud(pc_info, info, output));
+  }
+}
+
+// The encoder itself must refuse a field that does not fit in point_step,
+// whoever the caller is (PCL, Python, WASM bindings).
+TEST(Cloudini, EncoderRejectsFieldOutsidePointStep) {
+  Cloudini::EncodingInfo info;
+  info.fields = {{"x", 0, Cloudini::FieldType::FLOAT32, 0.001f}, {"y", 6, Cloudini::FieldType::FLOAT32, 0.001f}};
+  info.point_step = 8;  // y would span bytes 6..9
+  info.width = 10;
+  EXPECT_THROW(Cloudini::PointcloudEncoder encoder(info), std::runtime_error);
+}