diff --git a/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp b/dds/DCPS/transport/rtps_udp/RtpsSampleHeader.cpp index 7a5d9b9e753..18e164e342a 100644 --- 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 index 33800f99bdd..98c47308188 100644 --- 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 index ce203bd0d28..7935653b217 100644 --- 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 new file mode 100644 index 00000000000..d2fae4086cf --- /dev/null +++ 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 b/docs/news.d/rtps-parameterlist.rst similarity index 100% rename from rtps-parameterlist.rst rename to 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 new file mode 100644 index 00000000000..433045ff157 --- /dev/null +++ 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 + +#include + +#include + +using namespace OpenDDS::DCPS; + +namespace { + template + ACE_Message_Block* array_to_mb(const unsigned char (&array)[N]) + { + ACE_Message_Block* mb = new ACE_Message_Block(reinterpret_cast(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()); +} +