Merge pull request OpenDDS#4348 from mitza-oci/rtps-bad-datafrag

RtpsSampleHeader: reject invalid DataFrag submessages
iguessthislldo · Nov 13, 2023 · 230e75d · 230e75d
2 parents 569ce6c + 01a42a8
commit 230e75d
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 5 deletions.
diff --git a/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp b/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp
@@ -163,12 +163,19 @@ RtpsSampleHeader::init(ACE_Message_Block& mb)
       return;
     }
 
-    if ((kind == DATA && (flags & (FLAG_D | FLAG_K_IN_DATA)))
-        || kind == DATA_FRAG) {
+    if ((data_ && (flags & (FLAG_D | FLAG_K_IN_DATA))) || frag_) {
       // These Submessages have a payload which we haven't deserialized yet.
       // The TransportReceiveStrategy will know this via message_length().
       // octetsToNextHeader does not count the SubmessageHeader (4 bytes)
       message_length_ = octetsToNextHeader + SMHDR_SZ - serialized_size_;
+      if (frag_) {
+        const DataFragSubmessage& df = submessage_.data_frag_sm();
+        if (df.fragmentSize == 0 || df.fragmentStartingNum.value == 0 ||
+            df.fragmentsInSubmessage == 0 || last_fragment(df) > total_fragments(df)) {
+          valid_ = false;
+          return;
+        }
+      }
     } else {
       // These Submessages _could_ have extra data that we don't know about
       // (from a newer minor version of the RTPS spec).  Either way, indicate
@@ -765,6 +772,16 @@ RtpsSampleHeader::split(const ACE_Message_Block& orig, size_t size,
                        starting_frag + frags + tail_frags - 1);
 }
 
+FragmentNumber RtpsSampleHeader::last_fragment(const RTPS::DataFragSubmessage& df)
+{
+  return df.fragmentStartingNum.value + df.fragmentsInSubmessage - 1;
+}
+
+ACE_UINT32 RtpsSampleHeader::total_fragments(const RTPS::DataFragSubmessage& df)
+{
+  return df.sampleSize / df.fragmentSize + ((df.sampleSize % df.fragmentSize) ? 1 : 0);
+}
+
 }
 }
 

diff --git a/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.h b/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.h
@@ -97,9 +97,12 @@ class OpenDDS_Rtps_Udp_Export RtpsSampleHeader {
   // of <= (see issue 16966).
   static const ACE_CDR::UShort FRAG_SIZE = 1024;
 
+  static FragmentNumber last_fragment(const RTPS::DataFragSubmessage& df);
+  static ACE_UINT32 total_fragments(const RTPS::DataFragSubmessage& df);
+
 private:
   static void process_iqos(DataSampleHeader& opendds,
-                           const OpenDDS::RTPS::ParameterList& iqos);
+                           const RTPS::ParameterList& iqos);
 };
 
 }

diff --git a/dds/DCPS/transport/rtps_udp/RtpsUdpReceiveStrategy.cpp b/dds/DCPS/transport/rtps_udp/RtpsUdpReceiveStrategy.cpp
@@ -1043,9 +1043,9 @@ RtpsUdpReceiveStrategy::check_header(const RtpsSampleHeader& header)
   if (header.valid() && header.submessage_._d() == RTPS::DATA_FRAG) {
     const RTPS::DataFragSubmessage& rtps = header.submessage_.data_frag_sm();
     frags_.first = rtps.fragmentStartingNum.value;
-    frags_.second = frags_.first + (rtps.fragmentsInSubmessage - 1);
+    frags_.second = RtpsSampleHeader::last_fragment(rtps);
     fragment_size_ = rtps.fragmentSize;
-    total_frags_ = (rtps.sampleSize / rtps.fragmentSize) + (rtps.sampleSize % rtps.fragmentSize ? 1 : 0);
+    total_frags_ = RtpsSampleHeader::total_fragments(rtps);
   }
 
   return header.valid();

diff --git a/docs/news.d/rtps-badmsg.rst b/docs/news.d/rtps-badmsg.rst
@@ -0,0 +1,6 @@
+.. news-prs: 4348
+
+.. news-start-section: Fixes
+- Reject some types of invalid RTPS DataFrag submessages
+
+.. news-end-section
diff --git a/rtps-parameterlist.rst → docs/news.d/rtps-parameterlist.rst b/rtps-parameterlist.rst → docs/news.d/rtps-parameterlist.rst
diff --git a/tests/unit-tests/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp b/tests/unit-tests/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp
@@ -0,0 +1,105 @@
+/*
+ *
+ *
+ * Distributed under the OpenDDS License.
+ * See: http://www.opendds.org/license.html
+ */
+
+#include <gtest/gtest.h>
+
+#include <dds/DCPS/transport/rtps_udp/RtpsSampleHeader.h>
+
+#include <dds/DCPS/Message_Block_Ptr.h>
+
+using namespace OpenDDS::DCPS;
+
+namespace {
+  template <std::size_t N>
+  ACE_Message_Block* array_to_mb(const unsigned char (&array)[N])
+  {
+    ACE_Message_Block* mb = new ACE_Message_Block(reinterpret_cast<const char*>(array), N);
+    mb->wr_ptr(N);
+    return mb;
+  }
+}
+
+TEST(dds_DCPS_transport_rtps_udp_RtpsSampleHeader, DataFragValid)
+{
+  static const unsigned char x[] = {
+    0x16,0x01,0x00,0x00, // Submessage Header: DATA_FRAG, FLAG_E, 0 octets to next hdr
+    0x00,0x00,0x1c,0x00, // extraFlags, octetsToInlineQoS
+    0x00,0x00,0x00,0x00, // readerId
+    0x00,0x01,0x02,0x03, // writerId
+    0x00,0x00,0x00,0x00, // seq#-high
+    0x01,0x00,0x00,0x00, // seq#-low
+    0x01,0x00,0x00,0x00, // frag#-start
+    0x01,0x00,0x00,0x04, // numFrags, fragSize
+    0x99,0x00,0x00,0x00, // sampleSize
+  };
+  Message_Block_Ptr mb(array_to_mb(x));
+  RtpsSampleHeader rsh;
+  rsh.pdu_remaining(0x99 + sizeof x);
+  rsh = *mb;
+  ASSERT_TRUE(rsh.valid());
+}
+
+TEST(dds_DCPS_transport_rtps_udp_RtpsSampleHeader, DataFragInvalidSize)
+{
+  static const unsigned char x[] = {
+    0x16,0x01,0x00,0x00, // Submessage Header: DATA_FRAG, FLAG_E, 0 octets to next hdr
+    0x00,0x00,0x1c,0x00, // extraFlags, octetsToInlineQoS
+    0x00,0x00,0x00,0x00, // readerId
+    0x00,0x01,0x02,0x03, // writerId
+    0x00,0x00,0x00,0x00, // seq#-high
+    0x01,0x00,0x00,0x00, // seq#-low
+    0x01,0x00,0x00,0x00, // frag#-start
+    0x01,0x00,0x00,0x00, // numFrags, fragSize
+    0x99,0x00,0x00,0x00, // sampleSize
+  };
+  Message_Block_Ptr mb(array_to_mb(x));
+  RtpsSampleHeader rsh;
+  rsh.pdu_remaining(0x99 + sizeof x);
+  rsh = *mb;
+  ASSERT_FALSE(rsh.valid());
+}
+
+TEST(dds_DCPS_transport_rtps_udp_RtpsSampleHeader, DataFragInvalidStart)
+{
+  static const unsigned char x[] = {
+    0x16,0x01,0x00,0x00, // Submessage Header: DATA_FRAG, FLAG_E, 0 octets to next hdr
+    0x00,0x00,0x1c,0x00, // extraFlags, octetsToInlineQoS
+    0x00,0x00,0x00,0x00, // readerId
+    0x00,0x01,0x02,0x03, // writerId
+    0x00,0x00,0x00,0x00, // seq#-high
+    0x01,0x00,0x00,0x00, // seq#-low
+    0x00,0x00,0x00,0x00, // frag#-start
+    0x01,0x00,0x00,0x04, // numFrags, fragSize
+    0x99,0x00,0x00,0x00, // sampleSize
+  };
+  Message_Block_Ptr mb(array_to_mb(x));
+  RtpsSampleHeader rsh;
+  rsh.pdu_remaining(0x99 + sizeof x);
+  rsh = *mb;
+  ASSERT_FALSE(rsh.valid());
+}
+
+TEST(dds_DCPS_transport_rtps_udp_RtpsSampleHeader, DataFragInvalidEnd)
+{
+  static const unsigned char x[] = {
+    0x16,0x01,0x00,0x00, // Submessage Header: DATA_FRAG, FLAG_E, 0 octets to next hdr
+    0x00,0x00,0x1c,0x00, // extraFlags, octetsToInlineQoS
+    0x00,0x00,0x00,0x00, // readerId
+    0x00,0x01,0x02,0x03, // writerId
+    0x00,0x00,0x00,0x00, // seq#-high
+    0x01,0x00,0x00,0x00, // seq#-low
+    0x00,0x00,0x00,0x00, // frag#-start
+    0x02,0x00,0x00,0x04, // numFrags, fragSize
+    0x99,0x00,0x00,0x00, // sampleSize
+  };
+  Message_Block_Ptr mb(array_to_mb(x));
+  RtpsSampleHeader rsh;
+  rsh.pdu_remaining(0x99 + sizeof x);
+  rsh = *mb;
+  ASSERT_FALSE(rsh.valid());
+}
+