Skip to content

PayloadChannel Memory Corruption Cause Mediasoup Worker Terminate #570

@aggresss

Description

@aggresss

Bug Report

Your environment

  • Operating system: macOS x86 11.4
  • Node version:
  • npm version:
  • gcc/clang version:
  • mediasoup version: v3.7.6
  • mediasoup-client version:

Issue description

Use PayloadChannel send audio RTP payload may cause mediasoup worker terminate accidentally, like this:

PayloadChannel::UnixStreamSocket::UserOnUnixStreamRead() | NETSTRING_ERROR_NO_LENGTH
libc++abi: terminating with uncaught exception of type nlohmann::detail::parse_error: [json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - invalid literal; last read: ''

ConsumerSocket::UserOnUnixStreamRead() use its buffer to tirgger OnConsumerSocketMessage() and RtpPacket::Parse use as RtpPacket 's payload, packet->SetExtensions will shift the payload, this will occupy the memory which the next netstring in the payload buffer.

#569 may fix this issue.

Use this patch to detect:

diff --git a/worker/include/RTC/RtpPacket.hpp b/worker/include/RTC/RtpPacket.hpp
index 17d3aeea..187e0379 100644
--- a/worker/include/RTC/RtpPacket.hpp
+++ b/worker/include/RTC/RtpPacket.hpp
@@ -133,6 +133,25 @@ namespace RTC
 
 		static RtpPacket* Parse(const uint8_t* data, size_t len);
 
+	// Only for Memory Check Test
+	public:
+	static RtpPacket* ParseFromDirect(const uint8_t* data, size_t len) {
+            RtpPacket* pkt = RtpPacket::Parse(data, len);
+            pkt->isDirect = true;
+            pkt->magic = *(uint16_t*)(data+len);
+            return pkt;
+        }
+
+        bool MemoryCheck() {
+            if (this->isDirect) {
+                return this->magic == *(uint16_t*)(this->payload + this->payloadLength);
+            }
+            return true;
+        }
+	private:
+		bool isDirect { false };
+		uint16_t magic { 0 };
+
 	private:
 		RtpPacket(
 		  Header* header,
diff --git a/worker/src/RTC/DirectTransport.cpp b/worker/src/RTC/DirectTransport.cpp
index 00f63c87..c4007076 100644
--- a/worker/src/RTC/DirectTransport.cpp
+++ b/worker/src/RTC/DirectTransport.cpp
@@ -102,7 +102,7 @@ namespace RTC
 					return;
 				}
 
-				RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);
+				RTC::RtpPacket* packet = RTC::RtpPacket::ParseFromDirect(data, len);
 
 				if (!packet)
 				{
diff --git a/worker/src/RTC/Producer.cpp b/worker/src/RTC/Producer.cpp
index 96c35f8d..a0a4746b 100644
--- a/worker/src/RTC/Producer.cpp
+++ b/worker/src/RTC/Producer.cpp
@@ -1343,6 +1343,8 @@ namespace RTC
 			// Set the new extensions into the packet using One-Byte format.
 			packet->SetExtensions(1, extensions);
 
+			if (!packet->MemoryCheck()) {MS_ERROR("Memory Corruption Occurrence");}
+
 			// Assign mediasoup RTP header extension ids (just those that mediasoup may
 			// be interested in after passing it to the Router).
 			packet->SetMidExtensionId(static_cast<uint8_t>(RTC::RtpHeaderExtensionUri::Type::MID));

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions