From a737ecb2cbb56860f3f5ef8436958def84667995 Mon Sep 17 00:00:00 2001 From: Lee Thomason Date: Sat, 23 May 2026 16:36:17 -0700 Subject: [PATCH 1/2] Fixes from security review (#1067) Security fix --- tinyxml2.cpp | 21 ++++++++++++--------- xmltest.cpp | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index ad5bc87..c436fb2 100644 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -552,12 +552,15 @@ const char* XMLUtil::GetCharacterRef(const char* p, char* value, int* length) TIXMLASSERT(digit < radix); const unsigned int digitScaled = mult * digit; + // Reject before adding: if digitScaled alone exceeds MAX_CODE_POINT, + // or if adding it to ucs would exceed it (checked without overflow by + // testing ucs > MAX_CODE_POINT - digitScaled, safe since digitScaled + // <= MAX_CODE_POINT at this point). + if (digitScaled > MAX_CODE_POINT || ucs > MAX_CODE_POINT - digitScaled) { + return 0; + } ucs += digitScaled; - mult *= radix; - - // Security check: could a value exist that is out of range? - // Easily; limit to the MAX_CODE_POINT, which also allows for a - // bunch of leading zeroes. + mult *= radix; if (mult > MAX_CODE_POINT) { mult = MAX_CODE_POINT; } @@ -569,11 +572,11 @@ const char* XMLUtil::GetCharacterRef(const char* p, char* value, int* length) } // convert the UCS to UTF-8 ConvertUTF32ToUTF8(ucs, value, length); - if (length == 0) { - // If length is 0, there was an error. (Security? Bad input?) + if (*length == 0) { + // If *length is 0, ConvertUTF32ToUTF8 rejected the code point. // Fail safely. - return 0; - } + return 0; + } return p + delta + 1; } return p + 1; diff --git a/xmltest.cpp b/xmltest.cpp index d058707..f8e7196 100644 --- a/xmltest.cpp +++ b/xmltest.cpp @@ -2701,6 +2701,56 @@ int main( int argc, const char ** argv ) XMLTest("Test attribute encode with a Hex value", value5, "!"); // hex value in unicode value } + // ---------- Security: numeric character reference bounds ---------- + { + // Regression: U+10FFFF is the last valid Unicode code point and must + // parse correctly. The in-loop overflow guard must not reject it. + XMLDocument doc; + doc.Parse( "" ); + XMLTest( "Numeric ref U+10FFFF: no error", false, doc.Error() ); + const char* v = doc.FirstChildElement()->Attribute( "v" ); + // U+10FFFF encodes to the 4-byte UTF-8 sequence F4 8F BF BF. + const char expected[] = { + static_cast(0xF4), static_cast(0x8F), + static_cast(0xBF), static_cast(0xBF), 0 + }; + XMLTest( "Numeric ref U+10FFFF: correct UTF-8 output", expected, v ); + } + { + // Boundary check: U+110000 is one above the maximum code point. + // The in-loop overflow guard must catch this before ucs is written, + // leaving the entity as a literal (starting with '&'). + XMLDocument doc; + doc.Parse( "" ); + XMLTest( "Numeric ref U+110000: no parse error", false, doc.Error() ); + const char* v = doc.FirstChildElement()->Attribute( "v" ); + XMLTest( "Numeric ref U+110000: not resolved (left as literal)", true, + v != nullptr && v[0] == '&' ); + } + { + // A hex entity with enough digits to overflow uint32_t must + // be rejected by the in-loop guard before the accumulator wraps. + // Before the fix, ucs could wrap around and pass the post-loop range + // check, producing an attacker-chosen character in the parsed output. + // Build "&#x" + 300 'F' digits + ";" -- far beyond what fits in uint32_t. + const char prefix[] = ""; + static const int NDIGITS = 300; + char xml[sizeof(prefix) + NDIGITS + sizeof(suffix)]; + strcpy( xml, prefix ); + memset( xml + strlen(prefix), 'F', NDIGITS ); + strcpy( xml + strlen(prefix) + NDIGITS, suffix ); + + XMLDocument doc; + doc.Parse( xml ); + XMLTest( "Overflow hex entity: no parse error", false, doc.Error() ); + const char* v = doc.FirstChildElement()->Attribute( "v" ); + // GetCharacterRef returns 0 for rejected refs; the caller then copies + // the literal '&', so the attribute must start with '&', not a char. + XMLTest( "Overflow hex entity: not resolved to a character", true, + v != nullptr && v[0] == '&' ); + } + // ---------- XMLPrinter Apos Escaping ------ { const char* testText = "text containing a ' character"; From 999a21fa867c85da02d9d360b871bc9ee2b036ab Mon Sep 17 00:00:00 2001 From: Lee Thomason Date: Sat, 23 May 2026 16:41:46 -0700 Subject: [PATCH 2/2] Dead platform removal (#1068) Remove dead platforms. --- tinyxml2.cpp | 49 +++++++++---------------------------------------- tinyxml2.h | 5 +---- 2 files changed, 10 insertions(+), 44 deletions(-) diff --git a/tinyxml2.cpp b/tinyxml2.cpp index c436fb2..69d93ef 100644 --- a/tinyxml2.cpp +++ b/tinyxml2.cpp @@ -24,7 +24,7 @@ distribution. #include "tinyxml2.h" #include // yes, this one new style header, is in the Android SDK. -#if defined(ANDROID_NDK) || defined(__BORLANDC__) || defined(__QNXNTO__) || defined(__CC_ARM) +#if defined(ANDROID_NDK) # include # include #else @@ -40,9 +40,7 @@ distribution. # define __has_cpp_attribute(x) 0 #endif -#if defined(_MSC_VER) -# define TIXML_FALLTHROUGH (void(0)) -#elif (__cplusplus >= 201703L && __has_cpp_attribute(fallthrough)) +#if (__cplusplus >= 201703L && __has_cpp_attribute(fallthrough)) # define TIXML_FALLTHROUGH [[fallthrough]] #elif __has_cpp_attribute(clang::fallthrough) # define TIXML_FALLTHROUGH [[clang::fallthrough]] @@ -53,15 +51,8 @@ distribution. #endif -#if defined(_MSC_VER) && (_MSC_VER >= 1400 ) && (!defined WINCE) - // Microsoft Visual Studio, version 2005 and higher. Not WinCE. - /*int _snprintf_s( - char *buffer, - size_t sizeOfBuffer, - size_t count, - const char *format [, - argument] ... - );*/ +#if defined(_MSC_VER) && (_MSC_VER >= 1400) + // Microsoft Visual Studio, version 2005 and higher. static inline int TIXML_SNPRINTF( char* buffer, size_t size, const char* format, ... ) { va_list va; @@ -80,33 +71,11 @@ distribution. #define TIXML_VSCPRINTF _vscprintf #define TIXML_SSCANF sscanf_s #elif defined _MSC_VER - // Microsoft Visual Studio 2003 and earlier or WinCE + // Microsoft Visual Studio 2003 and earlier. #define TIXML_SNPRINTF _snprintf #define TIXML_VSNPRINTF _vsnprintf #define TIXML_SSCANF sscanf - #if (_MSC_VER < 1400 ) && (!defined WINCE) - // Microsoft Visual Studio 2003 and not WinCE. - #define TIXML_VSCPRINTF _vscprintf // VS2003's C runtime has this, but VC6 C runtime or WinCE SDK doesn't have. - #else - // Microsoft Visual Studio 2003 and earlier or WinCE. - static inline int TIXML_VSCPRINTF( const char* format, va_list va ) - { - int len = 512; - for (;;) { - len = len*2; - char* str = new char[len](); - const int required = _vsnprintf(str, len, format, va); - delete[] str; - if ( required != -1 ) { - TIXMLASSERT( required >= 0 ); - len = required; - break; - } - } - TIXMLASSERT( len >= 0 ); - return len; - } - #endif + #define TIXML_VSCPRINTF _vscprintf #else // GCC version 3 and higher //#warning( "Using sn* functions." ) @@ -124,7 +93,7 @@ distribution. #define TIXML_SSCANF sscanf #endif -#if defined(_WIN64) +#if defined(_MSC_VER) #define TIXML_FSEEK _fseeki64 #define TIXML_FTELL _ftelli64 #elif defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__) || defined(__CYGWIN__) @@ -2273,7 +2242,7 @@ void XMLDocument::Clear() delete [] _charBuffer; _charBuffer = 0; _parsingDepth = 0; - + #if 0 _textPool.Trace( "text" ); _elementPool.Trace( "element" ); @@ -2348,7 +2317,7 @@ static FILE* callfopen( const char* filepath, const char* mode ) { TIXMLASSERT( filepath ); TIXMLASSERT( mode ); -#if defined(_MSC_VER) && (_MSC_VER >= 1400 ) && (!defined WINCE) +#if defined(_MSC_VER) && (_MSC_VER >= 1400) FILE* fp = 0; const errno_t err = fopen_s( &fp, filepath, mode ); if ( err ) { diff --git a/tinyxml2.h b/tinyxml2.h index 723349e..048f854 100644 --- a/tinyxml2.h +++ b/tinyxml2.h @@ -24,15 +24,12 @@ distribution. #ifndef TINYXML2_INCLUDED #define TINYXML2_INCLUDED -#if defined(ANDROID_NDK) || defined(__BORLANDC__) || defined(__QNXNTO__) +#if defined(ANDROID_NDK) # include # include # include # include # include -# if defined(__PS3__) -# include -# endif #else # include # include