How Buffer Overflow via strcpy() Happens in C++ XML Parsers and How to Fix It

How Buffer Overflow via strcpy() Happens in C++ XML Parsers and How to Fix It

Introduction

The toXMLString function inside buildroot-external/package/libxmlparser/xmlParser.cpp has one job: convert special characters in XML content into their safe escape sequences. Characters like <, >, &, ', and " must become <, >, &, ', and " respectively. It sounds straightforward — but the original implementation used _tcscpy() to write those escape sequences into a destination buffer, with no check whatsoever on how much space remained. That single oversight created a critical buffer overflow that could allow an attacker to execute arbitrary code by feeding the parser a carefully crafted XML document.

This post walks through exactly how the vulnerability works, what the fix looks like at the code level, and what C++ developers can take away to avoid the same mistake in their own parsers and string-handling code.

The Vulnerability Explained

The Dangerous Pattern in toXMLString

Here is the vulnerable section of xmlParser.cpp (around line 156–165 in the original file):

// VULNERABLE CODE (before fix)
LPTSTR toXMLString(LPTSTR dest, LPCTSTR source)
{
    while (*source)
    {
        switch (*source)
        {
        case '<' : _tcscpy(dest, _T("&lt;"  )); dest += 4; break;
        case '>' : _tcscpy(dest, _T("&gt;"  )); dest += 4; break;
        case '&' : _tcscpy(dest, _T("&amp;" )); dest += 5; break;
        case '\'': _tcscpy(dest, _T("&apos;")); dest += 6; break;
        case '"' : _tcscpy(dest, _T("&quot;")); dest += 6; break;
        // ... default case handles regular characters
        }
        source++;
    }
}

The problem is _tcscpy(). This is the TCHAR-generic version of strcpy() — it copies a null-terminated string to the destination pointer and stops only when it hits \0. It does not accept a maximum length argument. It does not check whether dest has enough room.

The function receives a pre-allocated dest buffer from the caller, sized based on some estimate of the output length. If an attacker can cause the XML input to generate more escape sequences than the caller anticipated — for example, by packing a huge number of & characters into an element value — the writes through _tcscpy() will march right past the end of the allocated buffer.

How an Attacker Exploits This

The threat model notes that this is a local CLI tool where exploitation requires control over command-line arguments or input files. That means the attack surface is:

1. Malicious XML files passed as input — an attacker who can place or substitute an XML file that the tool processes (e.g., in a build pipeline, a shared directory, or via a supply-chain compromise of a configuration file) can trigger the overflow.
2. Embedded XML in buildroot packages — because this library lives in buildroot-external/package/libxmlparser/, it processes XML data as part of the build system. A compromised or crafted package definition could supply the malicious input.

A concrete exploitation scenario:

<!-- Crafted XML: 10,000 ampersands in a single attribute value -->
<config value="&amp;&amp;&amp;&amp;... (×10000) ...&amp;" />

Each & character becomes & (5 characters). If the caller allocated a buffer sized for, say, 1,024 characters of output, this input would cause toXMLString to write roughly 50,000 bytes — overflowing the buffer by ~49 KB. Depending on what lives adjacent to dest in memory, this could overwrite return addresses, function pointers, or other security-critical data, leading to arbitrary code execution.

Why _tcscpy() Is the Culprit

_tcscpy (and its siblings strcpy, wcscpy) are notorious in C/C++ security because:

• They have no length parameter — there is no way to tell them "stop after N bytes."
• They are unconditionally trusting — they will write as many bytes as the source string contains.
• The C standard itself has deprecated strcpy in favor of strcpy_s in C11, and major compilers emit warnings when they see it.

The five calls in the switch statement each copy a string literal of known, fixed length (4–6 TCHAR units). The literals themselves are safe — the danger is that the destination pointer dest is advanced through a buffer that may not be large enough to hold the fully escaped output for a large or adversarial input.

The Fix

Replacing _tcscpy() with Size-Bounded memcpy()

The fix is surgical and precise. Every _tcscpy() call is replaced with a memcpy() that specifies exactly how many bytes to copy:

// FIXED CODE (after fix)
case '<' : memcpy(dest, _T("&lt;"),   4*sizeof(TCHAR)); dest += 4; break;
case '>' : memcpy(dest, _T("&gt;"),   4*sizeof(TCHAR)); dest += 4; break;
case '&' : memcpy(dest, _T("&amp;"),  5*sizeof(TCHAR)); dest += 5; break;
case '\'': memcpy(dest, _T("&apos;"), 6*sizeof(TCHAR)); dest += 6; break;
case '"' : memcpy(dest, _T("&quot;"), 6*sizeof(TCHAR)); dest += 6; break;

Before vs. After

Why memcpy() Is the Right Tool Here

memcpy() copies exactly the number of bytes you specify — no more, no less. Since the escape sequences are string literals with known, compile-time-fixed lengths (4, 4, 5, 6, and 6 TCHAR units respectively), the byte counts are constants baked into the source code. There is no runtime uncertainty.

The sizeof(TCHAR) multiplier is important for correctness in Unicode builds: on Windows, TCHAR can be 2 bytes wide (wchar_t), so 4*sizeof(TCHAR) correctly copies 8 bytes for the 4-character string < in wide-character mode.

Supporting Header Additions

The fix also adds three #include directives that were previously missing:

#include <stdint.h>
#include <stddef.h>
#include <limits.h>

These headers provide size_t, ptrdiff_t, and integer limit constants — foundational types for safe size arithmetic in C/C++. Their absence was a latent risk: code that performs pointer arithmetic or size calculations without these types may silently use incorrect integer widths on some platforms.

Version Bump

The library version was incremented from 1.12 to 1.13 in both libxmlparser.mk and the file header comment. This is good practice: a version change signals to downstream consumers that the library has changed, enabling them to track the security fix in their dependency manifests.

Key Takeaways

• _tcscpy() in toXMLString() was copying escape sequences with no knowledge of how much buffer space remained — a textbook CWE-120 waiting to be exploited.
• The fix uses memcpy() with compile-time-constant byte counts — because the escape sequence lengths are fixed, memcpy(dest, _T("<"), 4*sizeof(TCHAR)) is both safe and efficient.
• sizeof(TCHAR) matters in XML parsers that support both narrow and wide characters — omitting it would silently under-copy in Unicode builds.
• Missing <stdint.h>, <stddef.h>, and <limits.h> is a warning sign — code that does pointer arithmetic without these headers may use incorrect integer types on some platforms.
• Buildroot external packages are production attack surface — even internal build tooling that processes XML from package definitions needs the same security rigor as user-facing code.

How Orbis AppSec Detected This

• Source: XML input data (element names, attribute values, text content) supplied to toXMLString() in xmlParser.cpp
• Sink: _tcscpy(dest, _T("<")) and four similar calls at lines ~159–163 of xmlParser.cpp, writing into a caller-managed destination buffer
• Missing control: No bounds check on dest before writing; no maximum-length parameter passed to the copy function; no assertion that the destination buffer has sufficient remaining capacity
• CWE: CWE-120 — Buffer Copy without Checking Size of Input
• Fix: Replaced all five _tcscpy() calls with memcpy() calls specifying exact byte counts (N*sizeof(TCHAR)), eliminating the possibility of writing beyond the intended copy length

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 from strcpy() and its TCHAR-generic cousin _tcscpy() have been a source of critical vulnerabilities for decades — and they keep appearing in production code because the functions are so easy to reach for. In xmlParser.cpp, the toXMLString function was doing exactly what its name promised, but the mechanism it used to write XML escape sequences could be weaponized by anyone who could feed the parser a large enough input.

The fix is a model of precision: five _tcscpy() calls replaced with five memcpy() calls, each specifying exactly the number of TCHAR units being copied. No behavior change, no performance regression, and the overflow path is closed permanently. Pair that with the addition of the missing safety headers and a version bump to signal the change, and you have a clean, reviewable, auditable security patch.

For developers maintaining C or C++ code that handles string data — especially in parsers, serializers, and format converters — the lesson is clear: treat every strcpy as a bug waiting to be filed. The safe alternatives (memcpy with explicit sizes, strlcpy, snprintf, or better yet std::string) cost almost nothing and eliminate an entire class of critical vulnerabilities.