Fix RTCP SDES (#1139)

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+### Next
+
+* Fix RTCP SDES ([PR #1139](https://github.com/versatica/mediasoup/pull/1139)).
+
+
 ### 3.12.8
 
 * Export `workerBin` absolute path ([PR #1123](https://github.com/versatica/mediasoup/pull/1123)).

worker/Dockerfile

@@ -12,7 +12,7 @@ RUN \
  clang
 
 # Install node 16.
-RUN curl -sL https://deb.nodesource.com/setup_16.x | bash -
+RUN curl -sL https://deb.nodesource.com/setup_18.x | bash -
 RUN apt-get install -y nodejs
 
 # Make CC and CXX point to clang/clang++ installed above.

worker/include/RTC/RTCP/Sdes.hpp

@@ -121,6 +121,9 @@ namespace RTC
  size += item->GetSize();
  }
 
+ // Add the mandatory null octet.
+ ++size;
+
  // Consider pading to 32 bits (4 bytes) boundary.
  // http://stackoverflow.com/questions/11642210/computing-padding-required-for-n-byte-alignment
  return (size + 3) & ~3;

worker/src/RTC/RTCP/Sdes.cpp

@@ -36,17 +36,21 @@ namespace RTC
  // Get the header.
  auto* header = const_cast<Header*>(reinterpret_cast<const Header*>(data));
 
+ // If item type is 0, there is no need for length field (unless padding
+ // is needed).
+ if (len > 0 && header->type == SdesItem::Type::END)
+ {
+ return nullptr;
+ }
+
  // data size must be >= header + length value.
- if (len < HeaderSize || len < (1u * 2) + header->length)
+ if (len < HeaderSize || len < HeaderSize + header->length)
  {
  MS_WARN_TAG(rtcp, "not enough space for SDES item, discarded");
 
  return nullptr;
  }
 
- if (header->type == SdesItem::Type::END)
- return nullptr;
-
  return new SdesItem(header);
  }
 
@@ -57,7 +61,9 @@ namespace RTC
  auto it = SdesItem::type2String.find(type);
 
  if (it == SdesItem::type2String.end())
+ {
  return Unknown;
+ }
 
  return it->second;
  }
@@ -123,19 +129,77 @@ namespace RTC
 
  std::unique_ptr<SdesChunk> chunk(new SdesChunk(ssrc));
 
- size_t offset = 4u /* ssrc */;
+ size_t offset{ 4u }; /* ssrc */
+ size_t chunkLength{ 4u };
 
  while (len > offset)
  {
- SdesItem* item = SdesItem::Parse(data + offset, len - offset);
+ auto* item = SdesItem::Parse(data + offset, len - offset);
 
  if (!item)
+ {
  break;
+ }
 
  chunk->AddItem(item);
+ chunkLength += item->GetSize();
  offset += item->GetSize();
  }
 
+ // Once all items have been parsed, there must be 1, 2, 3 or 4 null octets
+ // (this is mandatory). The first one (AKA item type 0) means "end of
+ // items", and the others maybe needed for padding the chunk to 4 bytes.
+
+ // First ensure that there is at least one null octet.
+ if (offset == len || (offset < len && Utils::Byte::Get1Byte(data, offset) != 0u))
+ {
+ MS_WARN_TAG(rtcp, "invalid SDES chunk (missing mandatory null octet), discarded");
+
+ return nullptr;
+ }
+
+ // So we have the mandatory null octet.
+ ++chunkLength;
+ ++offset;
+
+ // Then check that there are 0, 1, 2 or 3 (no more) null octets that pad
+ // the chunk to 4 bytes.
+ uint16_t neededAdditionalNullOctets =
+ Utils::Byte::PadTo4Bytes(static_cast<uint16_t>(chunkLength)) -
+ static_cast<uint16_t>(chunkLength);
+ uint16_t foundAdditionalNullOctets{ 0u };
+
+ for (uint16_t i{ 0u }; len > offset && i < neededAdditionalNullOctets; ++i)
+ {
+ if (Utils::Byte::Get1Byte(data, offset) != 0u)
+ {
+ MS_WARN_TAG(
+ rtcp,
+ "invalid SDES chunk (missing additional null octets [needed:%" PRIu16 ", found:%" PRIu16
+ "]), discarded",
+ neededAdditionalNullOctets,
+ foundAdditionalNullOctets);
+
+ return nullptr;
+ }
+
+ ++foundAdditionalNullOctets;
+ ++chunkLength;
+ ++offset;
+ }
+
+ if (foundAdditionalNullOctets != neededAdditionalNullOctets)
+ {
+ MS_WARN_TAG(
+ rtcp,
+ "invalid SDES chunk (missing additional null octets [needed:%" PRIu16 ", found:%" PRIu16
+ "]), discarded",
+ neededAdditionalNullOctets,
+ foundAdditionalNullOctets);
+
+ return nullptr;
+ }
+
  return chunk.release();
  }
 
@@ -155,10 +219,15 @@ namespace RTC
  offset += item->Serialize(buffer + offset);
  }
 
+ // Add the mandatory null octet.
+ buffer[offset] = 0;
+
+ ++offset;
+
  // 32 bits padding.
  const size_t padding = (-offset) & 3;
 
- for (size_t i{ 0 }; i < padding; ++i)
+ for (size_t i{ 0u }; i < padding; ++i)
  {
  buffer[offset + i] = 0;
  }
@@ -195,9 +264,9 @@ namespace RTC
  size_t offset = Packet::CommonHeaderSize;
  uint8_t count = header->count;
 
- while ((count-- != 0u) && (len > offset))
+ while (count-- && (len > offset))
  {
- SdesChunk* chunk = SdesChunk::Parse(data + offset, len - offset);
+ auto* chunk = SdesChunk::Parse(data + offset, len - offset);
 
  if (chunk != nullptr)
  {
@@ -206,10 +275,17 @@ namespace RTC
  }
  else
  {
- return packet.release();
+ break;
  }
  }
 
+ if (packet->GetCount() != header->count)
+ {
+ MS_WARN_TAG(rtcp, "RTCP count value doesn't match found number of chunks, discarded");
+
+ return nullptr;
+ }
+
  return packet.release();
  }
 
@@ -223,7 +299,7 @@ namespace RTC
  size_t length = 0;
  uint8_t* header = { nullptr };
 
- for (size_t i = 0; i < this->GetCount(); i++)
+ for (size_t i{ 0u }; i < this->GetCount(); ++i)
  {
  // Create a new SDES packet header for each 31 chunks.
  if (i % MaxChunksPerPacket == 0)