diff --git a/cloudini_lib/include/cloudini_lib/ros_message_definitions.hpp b/cloudini_lib/include/cloudini_lib/ros_message_definitions.hpp index 2863052..382694f 100644 --- a/cloudini_lib/include/cloudini_lib/ros_message_definitions.hpp +++ b/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 diff --git a/cloudini_lib/src/cloudini.cpp b/cloudini_lib/src/cloudini.cpp index b3f44b3..ed521f6 100644 --- a/cloudini_lib/src/cloudini.cpp +++ b/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(field.offset) + static_cast(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_)) { diff --git a/cloudini_lib/src/ros_msg_utils.cpp b/cloudini_lib/src/ros_msg_utils.cpp index 48ddb2e..3e5330b 100644 --- a/cloudini_lib/src/ros_msg_utils.cpp +++ b/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; diff --git a/cloudini_lib/test/test_header.cpp b/cloudini_lib/test/test_header.cpp index 1b78cd4..085d4ba 100644 --- a/cloudini_lib/test/test_header.cpp +++ b/cloudini_lib/test/test_header.cpp @@ -21,6 +21,7 @@ #include #include "cloudini_lib/cloudini.hpp" +#include "cloudini_lib/ros_message_definitions.hpp" // second includer: see test_ros_msg.cpp namespace { diff --git a/cloudini_lib/test/test_ros_msg.cpp b/cloudini_lib/test/test_ros_msg.cpp index d153ee9..95bec89 100644 --- a/cloudini_lib/test/test_ros_msg.cpp +++ b/cloudini_lib/test/test_ros_msg.cpp @@ -18,9 +18,11 @@ #include #include +#include #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 loadSampleDDSMessage() { + std::ifstream file(Cloudini::tests::DATA_PATH + "dds_message.bin", std::ios::binary); + EXPECT_TRUE(file.is_open()); + return std::vector((std::istreambuf_iterator(file)), std::istreambuf_iterator()); +} +} // 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 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); +}