How buffer overflow happens in C RTSPSession.h and how to fix it

How Buffer Overflow Happens in C RTSPSession.h and How to Fix It

Introduction

The RTSPSession.h file is the heart of an RTSP server implementation — it handles incoming client requests, parses headers, and manages transport negotiation. But a subtle flaw in how it copies incoming request data into an internal buffer created a critical security hole: an attacker who could send a single crafted network packet could potentially take over the entire process.

The culprit was a memcpy() call at line 408 with no bounds check — a pattern so common in C codebases that it often slips past code review unnoticed. This post walks through exactly what was wrong, how it could be exploited, and how the fix closes the door.

The Vulnerability Explained

What the code was doing

Inside RtspSession, incoming RTSP requests are stored in mCurRequest, a buffer with a fixed allocated capacity. When a new request arrives, the handler copies it in with:

// VULNERABLE — before the fix (RTSPSession.h, ~line 408)
const unsigned CurRequestSize = aRequestSize;
memcpy(mCurRequest.data(), aRequest, aRequestSize);

The problem is stark: aRequestSize is derived from the network — it reflects whatever size the client claims the request is. There is no check that aRequestSize <= mCurRequest.size() before the copy executes.

Why this is dangerous

memcpy() is a blunt instrument. It copies exactly as many bytes as you tell it to, regardless of whether the destination has room. When aRequestSize exceeds the capacity of mCurRequest, the function writes past the end of the buffer into adjacent heap memory. Depending on what lives there, this can:

• Corrupt heap metadata, causing a crash (denial of service)
• Overwrite adjacent objects, changing program logic
• In a carefully staged attack, overwrite function pointers or return addresses to redirect execution — remote code execution

The attack scenario

An attacker with network access to the RTSP server constructs a malformed RTSP request. The Content-Length or raw packet size is set to a value larger than mCurRequest's allocated capacity — say, 8 MB when the buffer holds 4 KB. The server receives the packet, reads aRequestSize directly from the stream, and calls:

memcpy(mCurRequest.data(), aRequest, 8388608); // buffer is only 4096 bytes

The overflow begins immediately, writing attacker-controlled bytes into the heap beyond mCurRequest. On a modern system with heap hardening this may crash the process; on a less-hardened target, or with careful heap grooming, it can yield a shell.

Additional unsafe string operations

The same file contained three more unsafe string operations that compounded the risk:

// strcpy with no length bound — destination size unknown to caller
strcpy(CP, ClientPortPtr);
strcpy(CP, eq);

// strncpy — truncates but does NOT guarantee null termination
strncpy(CP, TransportPtr, m_Response.size() - 1);
CP[m_Response.size() - 1] = '\0';

// Hard-coded limit of 256 — ignores actual buffer size
strncpy(m_Buf1.data(), m_URLHostPort.data(), 256);

Each of these is a potential overflow or truncation bug depending on input length and buffer layout.

The Fix

Primary fix: guard the memcpy

The core change is a single line inserted immediately before the memcpy:

// FIXED — RTSPSession.h, line 407 (after fix)
if (aRequestSize > mCurRequest.size()) return false;
const unsigned CurRequestSize = aRequestSize;
memcpy(mCurRequest.data(), aRequest, aRequestSize);

Before:

const unsigned CurRequestSize = aRequestSize;
memcpy(mCurRequest.data(), aRequest, aRequestSize);

After:

if (aRequestSize > mCurRequest.size()) return false;
const unsigned CurRequestSize = aRequestSize;
memcpy(mCurRequest.data(), aRequest, aRequestSize);

This is the minimal, correct fix. If the incoming size exceeds the buffer's actual capacity, the function returns false immediately — the oversized data is never touched. The check uses mCurRequest.size() (the actual runtime capacity of the container) rather than a hardcoded constant, so it remains correct even if the buffer size changes in the future.

Secondary fixes: replace strcpy/strncpy with snprintf

The three unsafe string operations were each replaced with snprintf, which always respects the destination size and guarantees null termination:

Client port parsing — before:

strcpy(CP, ClientPortPtr);
// ... later ...
strcpy(CP, eq);

After:

snprintf(CP, m_Response.size(), "%s", ClientPortPtr);
// ... later ...
snprintf(CP, m_Response.size(), "%s", eq);

Transport parsing — before:

strncpy(CP, TransportPtr, m_Response.size() - 1);
CP[m_Response.size() - 1] = '\0';

After:

snprintf(CP, m_Response.size(), "%s", TransportPtr);

Host/port parsing — before:

strncpy(m_Buf1.data(), m_URLHostPort.data(), 256);

After:

snprintf(m_Buf1.data(), m_Buf1.size(), "%s", m_URLHostPort.data());

Note the last change is doubly important: the original code used a hardcoded 256 as the limit, which would be wrong if m_Buf1 were ever resized. The fix uses m_Buf1.size() — the actual runtime size of the buffer — making it self-consistent.

Why these changes work together

The memcpy guard stops the most severe attack vector: an oversized request payload overflowing the main request buffer. The snprintf replacements close the secondary paths: even if an attacker gets past the first check (or exploits a different entry point), the string operations downstream can no longer be made to overflow their destinations.

Key Takeaways

• memcpy with an attacker-controlled size and no bounds check is a remote code execution primitive — the single missing guard in RTSPSession.h was all it took.
• The fix is one line, but the right one line: if (aRequestSize > mCurRequest.size()) return false; placed before the copy, using the runtime size of the actual buffer.
• strncpy is not a safe replacement for strcpy — it can leave buffers without null termination. snprintf(dest, dest_size, "%s", src) is the correct pattern.
• Hardcoded size limits like 256 in strncpy(m_Buf1.data(), ..., 256) are a maintenance hazard — always use the actual buffer's runtime size so the check stays correct if the buffer is resized.
• Network-facing C/C++ code deserves extra scrutiny on every copy operation — every memcpy, strcpy, strncpy, and sprintf that touches externally-supplied data is a potential vulnerability.

How Orbis AppSec Detected This

• Source: Incoming RTSP network request — the aRequest pointer and aRequestSize value are both attacker-controlled, arriving directly from the network socket.
• Sink: memcpy(mCurRequest.data(), aRequest, aRequestSize) at RTSPSession.h:408, where the attacker-controlled size is used without validation.
• Missing control: No check that aRequestSize <= mCurRequest.size() before the copy. The buffer's capacity was never consulted.
• CWE: CWE-120 — Buffer Copy without Checking Size of Input ("Classic Buffer Overflow")
• Fix: Inserted if (aRequestSize > mCurRequest.size()) return false; immediately before the memcpy, and replaced four unsafe strcpy/strncpy calls with bounded snprintf equivalents throughout the same file.

Orbis AppSec automatically detected this vulnerability and opened a pull request with the fix. Try Orbis AppSec on your repositories to find and fix issues like this automatically.

Conclusion

Buffer overflows in C are old, well-understood, and still showing up in production network code in 2024. The vulnerability in RTSPSession.h is a textbook example: a single memcpy call that trusts the caller to supply a safe size. The fix is equally textbook — validate first, copy second. What makes this case instructive is the compound nature of the problem: the primary memcpy overflow was accompanied by three additional unsafe string operations in the same file, each one a separate path to memory corruption. Fixing the whole class of issues together, using snprintf with runtime buffer sizes, is the right approach.

If you maintain C or C++ code that handles network input, audit every memcpy, strcpy, strncpy, and sprintf call that touches externally-supplied data. The check is cheap; the exploit is not.