Heap Buffer Overflow in XML Text Escaping

[cktii]

Description

NOTE: I was testing Commit 5ddde9ff91565e17d99d9b827e29177e33025975

Buffer overflow in CMarkup::x_TextToDoc when escaping XML special characters. The function incorrectly uses total buffer size instead of remaining space when copying escape sequences.

Affected Functions

• CMarkup::x_TextToDoc - Source
• Called through CRTProtocol::SetSkeletonSettings

Impact

• Memory corruption
• Potential code execution
• Crash (DoS)

Stack Trace

ASAN Report:
=================================================================
==2223285==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x504000000031 at pc 0x55555563fdf7 bp 0x7fffffffd270 sp 0x7fffffffca28
WRITE of size 32 at 0x504000000031 thread T0
    #0 0x55555563fdf6 in strncpy ??:?
    #1 0x5555556ddb2f in CMarkup::x_TextToDoc[abi:cxx11](char const*, bool) const /home/user/qualisys_cpp_sdk/Markup.cpp:734
    #2 0x5555556dbc28 in CMarkup::x_SetAttrib(int, char const*, char const*) /home/user/qualisys_cpp_sdk/Markup.cpp:655
    #3 0x5555557a2034 in CRTProtocol::SetSkeletonSettings(std::vector<CRTProtocol::SSettingsSkeletonHierarchical, std::allocator<CRTProtocol::SSettingsSkeletonHierarchical> > const&) /home/user/qualisys_cpp_sdk/RTProtocol.cpp:5928
    #4 0x5555556a06c0 in LLVMFuzzerTestOneInput /home/user/qualisys_cpp_sdk/sdk_fuzz.cc:147
    #5 0x555555864349 in ExecuteFilesOnyByOne /home/user/AFLplusplus/utils/aflpp_driver/aflpp_driver.c:255
    #6 0x555555864145 in LLVMFuzzerRunDriver /home/user/AFLplusplus/utils/aflpp_driver/aflpp_driver.c:?
    #7 0x555555863cfd in main /home/user/AFLplusplus/utils/aflpp_driver/aflpp_driver.c:311
    #8 0x7ffff782a1c9 in __libc_start_call_main csu/../sysdeps/x86/libc-start.c:58
    #9 0x7ffff782a28a in __libc_start_main_impl csu/../csu/libc-start.c:360
    #10 0x5555555bc874 in _start ??:?

0x504000000031 is located 0 bytes after 33-byte region [0x504000000010,0x504000000031)
allocated by thread T0 here:
    #0 0x555555695ce1 in operator new(unsigned long) ??:?
    #1 0x7ffff7d6870e in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned long, unsigned long, char const*, unsigned long) ??:?
    #2 0x7ffff7d694ff in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_replace_aux(unsigned long, unsigned long, unsigned long, char) ??:?
    #3 0x5555556dd481 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::resize(unsigned long) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/basic_string.h:1114
    #4 0x5555556dd481 in CMarkup::GetBuffer(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, int) const /home/user/qualisys_cpp_sdk/Markup.cpp:1236
    #5 0x5555556dd481 in CMarkup::x_TextToDoc[abi:cxx11](char const*, bool) const /home/user/qualisys_cpp_sdk/Markup.cpp:716
    #6 0x5555556dbc28 in CMarkup::x_SetAttrib(int, char const*, char const*) /home/user/qualisys_cpp_sdk/Markup.cpp:655
    #7 0x5555557a2034 in CRTProtocol::SetSkeletonSettings(std::vector<CRTProtocol::SSettingsSkeletonHierarchical, std::allocator<CRTProtocol::SSettingsSkeletonHierarchical> > const&) /home/user/qualisys_cpp_sdk/RTProtocol.cpp:5928
    #8 0x5555556a06c0 in LLVMFuzzerTestOneInput /home/user/qualisys_cpp_sdk/sdk_fuzz.cc:147
    #9 0x555555864349 in ExecuteFilesOnyByOne /home/user/AFLplusplus/utils/aflpp_driver/aflpp_driver.c:255

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:? in strncpy
Shadow bytes around the buggy address:
  0x503ffffffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x503ffffffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x503ffffffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x503fffffff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x503fffffff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x504000000000: fa fa 00 00 00 00[01]fa fa fa fa fa fa fa fa fa
  0x504000000080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x504000000100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x504000000180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x504000000200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x504000000280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==2223285==ABORTING

Detailed stack trace

[...]
#10 0x00005555556ddb30 in CMarkup::x_TextToDoc[abi:cxx11](char const*, bool) const (/home/user/qualisys_cpp_sdk/build/sdk_fuzz_afl)
                       695: std::string CMarkup::x_TextToDoc[abi:cxx11](char const*, bool) const(this = (const class CMarkup *)0x7ffff5b09020, szText = (const char *)0x5030000000a0 "=w==>fuF<\a\212\016w==>fuFFFF\205", bAttrib = (bool)<optimized out>) {
                       |||:
                       |||: /* Local reference: char * pDest = 0x504000000010 "=w=="; */
                       |||: /* Local reference: int nLen = 4; */
                       |||: /* Local reference: int nDestSize = 32; */
                       |||: /* Local reference: const char * pFound = 0x5555558801e0 <str> "&gt;"; */
                       732:                     strcpy_s(&pDest[nLen], nDestSize, pFound);
                       733: #else
                       734:                     strncpy(&pDest[nLen], pFound, nDestSize);
                       |||:
                       ---: }
                       at /home/user/qualisys_cpp_sdk/Markup.cpp:734

#11 0x00005555556dbc29 in CMarkup::x_SetAttrib (/home/user/qualisys_cpp_sdk/build/sdk_fuzz_afl)
                       624: bool CMarkup::x_SetAttrib(this = (class CMarkup *)0x7ffff5b09020, iPos = (int)<optimized out>, szAttrib = (const char *)<optimized out>, szValue = (const char *)<optimized out>) {
                       |||:
                       |||: /* Local reference: std::string csFormat = " Name=\""; */
                       |||: /* Local reference: const char * szAttrib = <optimized out>; */
                       |||: /* Local reference: const char * szValue = <optimized out>; */
                       653:             csFormat += szAttrib;
                       654:             csFormat += "=\"";
                       655:             csFormat += x_TextToDoc(szValue, true);
                       |||:
                       ---: }
                       at /home/user/qualisys_cpp_sdk/Markup.cpp:655

#12 0x00005555557a2035 in CRTProtocol::SetSkeletonSettings (/home/user/qualisys_cpp_sdk/build/sdk_fuzz_afl)
                       5916: bool CRTProtocol::SetSkeletonSettings(this = (class CRTProtocol *)<optimized out>, skeletons = (const class std::vector<CRTProtocol::SSettingsSkeletonHierarchical, std::allocator<CRTProtocol::SSettingsSkeletonHierarchical> > &)<optimized out>) {
                       ||||:
                       ||||: /* Local reference: class CMarkup xml = {_vptr$CMarkup = <optimized out>, m_csDoc = , m_csError = , m_aPos = , m_iPosParent = <optimized out>, m_iPos = <optimized out>, m_iPosChild = <optimized out>, m_iPosFree = <optimi... */
                       ||||: /* Local reference: const struct CRTProtocol::SSettingsSkeletonHierarchical & skeleton = @0x517000000080; */
                       5926:     {
                       5927:         xml.AddElem("Skeleton");
                       5928:         xml.SetAttrib("Name", skeleton.name.c_str());
                       ||||:
                       ----: }
                       at /home/user/qualisys_cpp_sdk/RTProtocol.cpp:5928

[...]

Additional note:

There appears to be an undefined behavior vulnerability in x_TextToDoc through modification of string data returned by c_str(). The GetBuffer function (Ref.) uses const_cast to remove const from the pointer returned by std::string::c_str() and then modifies the underlying character array through this pointer. This violates the C++ standard requirement that "The program shall not modify any of the values stored in the character array" returned by c_str() (Ref.). This undefined behavior can lead to memory corruption, as the character array is not guaranteed to be modifiable and may be shared or read-only depending on the implementation.

Recommendation

Replace unsafe buffer manipulation with proper string handling as:

• strncpy was using total buffer size (nDestSize) instead of remaining space
• No length check to ensure the replacement text would fit
• Not accounting for null termination space

// Instead of manual buffer manipulation:
strncpy(&pDest[nLen], pFound, nDestSize);

// Use std::string operations:
std::string result;
result.reserve(strlen(szText) + strlen(szText)/10 + 7);  // Pre-allocate space
for (const char* p = szText; *p; ++p) {
    if (const char* found = strchr(pFind, *p)) {
        result += szaReplace[found - pFind];
    } else {
        result += *p;
    }
}

[cktii]

This issue has been assigned Use CVE-2024-53319