fix(parser_ros): bound-check CDR byte-sequence length to prevent OOB read

parser_ros/ros_builtin_object_handlers.cpp

@@ -115,7 +115,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseImage(PJ::Timestamp ts, PJ::
     deserializer_->deserializeString(encoding);
     const uint8_t is_be = readU8(*deserializer_);
     const uint32_t step = deserializer_->deserializeUInt32();
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
 
     auto it = kRosImageBytesPerPixel().find(encoding);
     if (it == kRosImageBytesPerPixel().end()) {
@@ -170,7 +170,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseCompressedImage(PJ::Timestam
 
     std::string format;
     deserializer_->deserializeString(format);
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
     const uint8_t* src = data_span.data();
     const uint32_t data_len = static_cast<uint32_t>(data_span.size());
 
@@ -334,7 +334,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseCompressedVideo(PJ::Timestam
 
     std::string frame_id;
     deserializer_->deserializeString(frame_id);
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
     std::string format;
     deserializer_->deserializeString(format);
 
@@ -408,7 +408,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parsePointCloud(PJ::Timestamp ts,
     const uint8_t is_be = readU8(*deserializer_);
     const uint32_t point_step = deserializer_->deserializeUInt32();
     const uint32_t row_step = deserializer_->deserializeUInt32();
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
 
     // is_dense follows data[]. deserializeByteSequence advanced the cursor
     // past the body, so the next byte read is is_dense itself.
@@ -574,7 +574,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseFoxgloveCompressedPointCloud
       (void)deserializer_->deserialize(RosMsgParser::FLOAT64);
     }
 
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
     std::string format;
     deserializer_->deserializeString(format);
 
@@ -639,7 +639,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseCompressedPointCloud2(
     (void)readU8(*deserializer_);              // is_bigendian
     (void)deserializer_->deserializeUInt32();  // point_step
     (void)deserializer_->deserializeUInt32();  // row_step
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
     (void)readU8(*deserializer_);  // is_dense
 
     std::string format;
@@ -1016,7 +1016,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseOccupancyGrid(PJ::Timestamp
     const double oz = deserializer_->deserialize(RosMsgParser::FLOAT64).convert<double>();
     const double ow = deserializer_->deserialize(RosMsgParser::FLOAT64).convert<double>();
 
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
 
     PJ::sdk::OccupancyGrid grid;
     grid.timestamp_ns = current_timestamp_;
@@ -1066,7 +1066,7 @@ PJ::Expected<PJ::sdk::ObjectRecord> RosParser::parseOccupancyGridUpdate(
     const int32_t y = deserializer_->deserialize(RosMsgParser::INT32).convert<int32_t>();
     const uint32_t width = deserializer_->deserializeUInt32();
     const uint32_t height = deserializer_->deserializeUInt32();
-    const auto data_span = deserializer_->deserializeByteSequence();
+    const auto data_span = readByteSequence();
 
     const size_t expected = static_cast<size_t>(width) * static_cast<size_t>(height);
     if (data_span.size() < expected) {

parser_ros/ros_marker_handlers.cpp

@@ -163,7 +163,7 @@ void RosParser::decodeOneMarker(PJ::sdk::SceneEntities& out) {
     deserializer_->deserializeString(texture_frame);
     std::string texture_format;
     deserializer_->deserializeString(texture_format);
-    (void)deserializer_->deserializeByteSequence();  // texture.data
+    (void)readByteSequence();  // texture.data
     const uint32_t num_uv = deserializer_->deserializeUInt32();
     if (num_uv > kMaxMarkerVertices) {
       throw std::runtime_error("Marker uv_coordinates[] exceeds sanity cap");
@@ -185,7 +185,7 @@ void RosParser::decodeOneMarker(PJ::sdk::SceneEntities& out) {
     // The byte sequence must be consumed unconditionally to keep the wire
     // aligned, but only a MESH_RESOURCE marker uses it — skip the copy of a
     // potentially multi-MB payload for every other marker type.
-    const auto bytes = deserializer_->deserializeByteSequence();  // mesh_file.data
+    const auto bytes = readByteSequence();  // mesh_file.data
     if (type == marker_type::kMeshResource) {
       mesh_file_data.assign(bytes.begin(), bytes.end());
     }

parser_ros/ros_parser.cpp

@@ -1,7 +1,9 @@
 #include <cctype>
+#include <cstdint>
 #include <cstdlib>
 #include <functional>
 #include <pj_array_policy/array_policy.hpp>
+#include <stdexcept>
 #include <string>
 
 #include "ros_parser_internal.hpp"
@@ -626,6 +628,21 @@ void RosParser::ensureDeserializer() {
   }
 }
 
+RosMsgParser::Span<const uint8_t> RosParser::readByteSequence() {
+  // Bytes available at the cursor, captured before the read (the 4-byte length
+  // prefix and any trailing fields are included, so the bound is conservative:
+  // it can only over-accept by that slack, never falsely reject a full message).
+  const size_t available = deserializer_->bytesLeft();
+  const auto span = deserializer_->deserializeByteSequence();
+  if (available < sizeof(uint32_t) || span.size() > available - sizeof(uint32_t)) {
+    // The declared length ran past the payload. deserializeByteSequence has already
+    // advanced the cursor past the end, so both this span AND any field the handler
+    // reads next would be out of bounds; abort now (caught upstream -> Expected error).
+    throw std::runtime_error("CDR byte sequence length exceeds message payload (truncated or corrupt message)");
+  }
+  return span;
+}
+
 void RosParser::detectSchemaFeatures() {
   const auto& schema = parser_->getSchema();
   const auto& root_fields = schema->root_msg->fields();

parser_ros/ros_parser_internal.hpp

@@ -254,6 +254,19 @@ class RosParser : public PJ::MessageParserPluginBase {
   // overrides (e.g. std_msgs/String on a robot_description topic -> RobotDescription).
   CatalogEntry selectCatalogEntry(const std::string& msg_type) const;
   void ensureDeserializer();
+  // Reads a CDR byte sequence (uint8[] / sequence<uint8>) and validates its
+  // wire-declared length against the bytes remaining in the message. NanoCDR's
+  // byte-sequence reader trusts the declared length and does not clamp it to the
+  // buffer (unlike the ROS1 deserializer and NanoCDR's own string reader): on a
+  // truncated/corrupt message it advances the cursor past the end and returns a span
+  // longer than the payload. Reading that span — or any field the handler reads
+  // after it, since the cursor is now past the end — is an out-of-bounds access.
+  // Throws std::runtime_error on overrun (matching the ROS1 reader) so the decode is
+  // aborted at once. Every object handler wraps its body in try/catch and the scalar
+  // void-handler path is guarded in wrapVoidHandler(), so the throw becomes an
+  // Expected error (no frame, no crash), never a std::terminate across the C ABI.
+  // Read all bulk uint8[] fields through this, never the deserializer directly.
+  RosMsgParser::Span<const uint8_t> readByteSequence();
   void detectSchemaFeatures();
   void findQuaternionPrefixes(
       const RosMsgParser::ROSMessage* msg, const std::string& prefix, const RosMsgParser::RosMessageLibrary& lib);
@@ -443,7 +456,15 @@ class RosParser : public PJ::MessageParserPluginBase {
     named_fields_.clear();
     current_timestamp_ = ts;
     deserializer_->init(RosMsgParser::Span<const uint8_t>(payload.data(), payload.size()));
-    (this->*Handler)();
+    // The object handlers each carry their own try/catch; the scalar void handlers
+    // do not, and this wrapper is the C-ABI-facing entry for them. A CDR decode
+    // throw (e.g. readByteSequence / deserializeString on a truncated message) must
+    // become an Expected error here, never propagate into the noexcept trampoline.
+    try {
+      (this->*Handler)();
+    } catch (const std::exception& e) {
+      return PJ::unexpected(std::string("ROS scalar decode error: ") + e.what());
+    }
     std::vector<PJ::sdk::NamedFieldValue> out;
     out.reserve(owned_fields_.size());
     for (const auto& f : owned_fields_) {

parser_ros/ros_specializations.cpp

@@ -267,8 +267,8 @@ void RosParser::handleDataTamerSnapshot() {
   uint64_t timestamp = deserializer_->deserialize(RosMsgParser::UINT64).convert<uint64_t>();
   uint64_t schema_hash = deserializer_->deserialize(RosMsgParser::UINT64).convert<uint64_t>();
 
-  auto active_mask = deserializer_->deserializeByteSequence();
-  auto payload = deserializer_->deserializeByteSequence();
+  auto active_mask = readByteSequence();
+  auto payload = readByteSequence();
 
   auto it = g_data_tamer_schemas.find(schema_hash);
   if (it == g_data_tamer_schemas.end()) {

parser_ros/tests/ros_parser_test.cpp

@@ -779,6 +779,24 @@ TEST(RosParserTest, PoseWithRPY) {
   EXPECT_NE(findField(f.recorder.rows()[0], "/orientation/pitch"), nullptr);
 }
 
+// Regression (scalar route): a truncated message whose scalar (void) handler reads
+// past the end must fail cleanly, not std::terminate. The void handlers run through
+// wrapVoidHandler(), which — unlike the object handlers — is the C-ABI-facing entry
+// for the scalar route, so it must catch the CDR decode throw before it crosses the
+// noexcept trampoline. Here geometry_msgs/Pose carries only position (3 doubles);
+// handlePose throws reading the missing orientation.
+TEST(RosParserTest, TruncatedScalarMessageDoesNotTerminate) {
+  RosParserFixture f;
+  f.setUp();
+  ASSERT_TRUE(f.bindSchema("geometry_msgs/Pose", kPoseDef));
+
+  auto payload = serializeCdr([](RosMsgParser::NanoCDR_Serializer& enc) {
+    serializeVector3(enc, 1.0, 2.0, 3.0);  // position only; orientation truncated away
+  });
+
+  EXPECT_FALSE(f.parse(payload)) << "truncated scalar message must fail cleanly, not terminate";
+}
+
 TEST(RosParserTest, ImuRPY) {
   RosParserFixture f;
   f.setUp();
@@ -1565,6 +1583,45 @@ TEST(RosParserTest, ImageObjectCarriesFrameId) {
   EXPECT_EQ(img->encoding, "mono8");
 }
 
+// Regression: a truncated sensor_msgs/Image whose data[] declares more bytes than
+// the message carries must be REJECTED, not decoded into a span that runs past the
+// payload. NanoCDR's byte-sequence reader does not clamp the declared length to the
+// buffer, so readByteSequence() adds the bound check the ROS1 path already has.
+// Without the fix, parseImage would emit an Image whose data span extends ~82 MB
+// past a 48-byte payload and the 2D viewer's copy would read off the end (SIGSEGV).
+TEST(RosParserTest, ImageWithTruncatedDataIsRejected) {
+  static const char* kImageDef =
+      "std_msgs/Header header\nuint32 height\nuint32 width\nstring encoding\n"
+      "uint8 is_bigendian\nuint32 step\nuint8[] data\n"
+      "================\nMSG: std_msgs/Header\nbuiltin_interfaces/Time stamp\nstring frame_id\n"
+      "================\nMSG: builtin_interfaces/Time\nint32 sec\nuint32 nanosec\n";
+
+  RosParserFixture f;
+  f.setUp();
+  ASSERT_TRUE(f.bindSchema("sensor_msgs/Image", kImageDef));
+
+  // Declare a data[] of step*height bytes but write NONE of them: the message ends
+  // right after the count. Mirrors a partially-written / corrupt bag.
+  constexpr uint32_t kHeight = 20000;
+  constexpr uint32_t kWidth = 4096;
+  constexpr uint32_t kStep = 4096;  // mono8 -> step == width*bpp
+  auto payload = serializeCdr([](RosMsgParser::NanoCDR_Serializer& enc) {
+    serializeHeader(enc, 7, 0, "camera_link");
+    enc.serializeUInt32(kHeight);
+    enc.serializeUInt32(kWidth);
+    enc.serializeString("mono8");
+    enc.serialize(RosMsgParser::UINT8, RosMsgParser::Variant(static_cast<uint8_t>(0)));  // is_bigendian
+    enc.serializeUInt32(kStep);
+    enc.serializeUInt32(kStep * kHeight);  // data[] count -- but no bytes follow
+  });
+
+  auto* base = static_cast<PJ::MessageParserPluginBase*>(f.handle.context());
+  ASSERT_NE(base, nullptr);
+  const PJ::sdk::PayloadView view{PJ::Span<const uint8_t>(payload.data(), payload.size()), {}};
+  auto rec = base->parseObject(1234, view);
+  EXPECT_FALSE(rec.has_value()) << "truncated Image must be rejected, not decoded into an out-of-bounds span";
+}
+
 // Regression: ROS Bayer CFA images (bayer_rggb8 and friends) carry one raw mosaic
 // sample per pixel (1 byte/pixel). parseImage must ACCEPT them and pass the encoding
 // through verbatim — the viewer demosaics downstream. Previously these were rejected