improve fix

update GitHub actions checkout and actions-setup-cmake

.github/workflows/test.yml

@@ -31,10 +31,10 @@ jobs:
     runs-on: ${{ matrix.os }}
     steps:
       # System set-up
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - uses: ilammy/msvc-dev-cmd@v1
       - uses: seanmiddleditch/gha-setup-ninja@master
-      - uses: jwlawson/actions-setup-cmake@v1.8
+      - uses: jwlawson/actions-setup-cmake@v2
         with:
           cmake-version: ${{ matrix.cmake }}
 

CMakeLists.txt

@@ -63,7 +63,9 @@ endif ()
 include(GNUInstallDirs)
 include(CMakePackageConfigHelpers)
 
-## Custom locations
+## Custom settings
+option(tinyxml2_INSTALL_PKGCONFIG "Create and install pkgconfig files" ON)
+
 set(tinyxml2_INSTALL_PKGCONFIGDIR "${CMAKE_INSTALL_LIBDIR}/pkgconfig"
     CACHE PATH "Directory for pkgconfig files")
 
@@ -121,10 +123,12 @@ install(
 
 ## pkg-config
 
-configure_file(cmake/tinyxml2.pc.in tinyxml2.pc.gen @ONLY)
-file(GENERATE OUTPUT tinyxml2.pc INPUT "${CMAKE_CURRENT_BINARY_DIR}/tinyxml2.pc.gen")
-install(
-    FILES "${CMAKE_CURRENT_BINARY_DIR}/tinyxml2.pc"
-    DESTINATION "${tinyxml2_INSTALL_PKGCONFIGDIR}"
-    COMPONENT tinyxml2_development
-)
+if (tinyxml2_INSTALL_PKGCONFIG)
+    configure_file(cmake/tinyxml2.pc.in tinyxml2.pc.gen @ONLY)
+    file(GENERATE OUTPUT tinyxml2.pc INPUT "${CMAKE_CURRENT_BINARY_DIR}/tinyxml2.pc.gen")
+    install(
+        FILES "${CMAKE_CURRENT_BINARY_DIR}/tinyxml2.pc"
+        DESTINATION "${tinyxml2_INSTALL_PKGCONFIGDIR}"
+        COMPONENT tinyxml2_development
+    )
+endif ()

contrib/html5-printer.cpp

@@ -7,7 +7,7 @@
 
 //  In HTML5, there are 16 so-called "void" elements.  "void elements" NEVER have
 //  inner content (but they MAY have attributes), and are assumed to be self-closing.
-//  An example of a self-closig HTML5 element is "<br/>" (line break)
+//  An example of a self-closing HTML5 element is "<br/>" (line break)
 //  All other elements are called "non-void" and MUST never self-close.
 //  Examples: "<div class='lolcats'></div>".
 

tinyxml2.cpp

@@ -106,14 +106,9 @@ distribution.
 #elif defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__NetBSD__) || defined(__DragonFly__) || defined(__CYGWIN__)
 	#define TIXML_FSEEK fseeko
 	#define TIXML_FTELL ftello
-#elif defined(__ANDROID__) 
-    #if __ANDROID_API__ > 24
-        #define TIXML_FSEEK fseeko64
-        #define TIXML_FTELL ftello64
-    #else
-        #define TIXML_FSEEK fseeko
-        #define TIXML_FTELL ftello
-    #endif
+#elif defined(__ANDROID__) && __ANDROID_API__ > 24
+	#define TIXML_FSEEK fseeko64
+	#define TIXML_FTELL ftello64
 #else
 	#define TIXML_FSEEK fseek
 	#define TIXML_FTELL ftell
@@ -239,13 +234,13 @@ char* StrPair::ParseName( char* p )
     if ( !p || !(*p) ) {
         return 0;
     }
-    if ( !XMLUtil::IsNameStartChar( (unsigned char) *p ) ) {
+    if ( !XMLUtil::IsNameStartChar( static_cast<unsigned char>(*p) ) ) {
         return 0;
     }
 
     char* const start = p;
     ++p;
-    while ( *p && XMLUtil::IsNameChar( (unsigned char) *p ) ) {
+    while ( *p && XMLUtil::IsNameChar( static_cast<unsigned char>(*p) ) ) {
         ++p;
     }
 
@@ -472,102 +467,89 @@ void XMLUtil::ConvertUTF32ToUTF8( unsigned long input, char* output, int* length
 }
 
 
-const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
+const char* XMLUtil::GetCharacterRef(const char* p, char* value, int* length)
 {
-    // Presume an entity, and pull it out.
+    // Assume an entity, and pull it out.
     *length = 0;
 
-    if ( *(p+1) == '#' && *(p+2) ) {
-        unsigned long ucs = 0;
-        TIXMLASSERT( sizeof( ucs ) >= 4 );
+    static const uint32_t MAX_CODE_POINT = 0x10FFFF;
+
+    if (*(p + 1) == '#' && *(p + 2)) {
+        uint32_t ucs = 0;
         ptrdiff_t delta = 0;
-        unsigned mult = 1;
+        uint32_t mult = 1;
         static const char SEMICOLON = ';';
 
-        if ( *(p+2) == 'x' ) {
+        bool hex = false;
+        uint32_t radix = 10;
+        const char* q = 0;
+        char terminator = '#';
+
+        if (*(p + 2) == 'x') {
             // Hexadecimal.
-            const char* q = p+3;
-            if ( !(*q) ) {
-                return 0;
-            }
+            hex = true;
+            radix = 16;
+            terminator = 'x';
 
-            q = strchr( q, SEMICOLON );
-
-            if ( !q ) {
-                return 0;
-            }
-            TIXMLASSERT( *q == SEMICOLON );
-
-            delta = q-p;
-            --q;
-
-            while ( *q != 'x' ) {
-                unsigned int digit = 0;
-
-                if ( *q >= '0' && *q <= '9' ) {
-                    digit = *q - '0';
-                }
-                else if ( *q >= 'a' && *q <= 'f' ) {
-                    digit = *q - 'a' + 10;
-                }
-                else if ( *q >= 'A' && *q <= 'F' ) {
-                    digit = *q - 'A' + 10;
-                }
-                else {
-                    return 0;
-                }
-                TIXMLASSERT( digit < 16 );
-                TIXMLASSERT( digit == 0 || mult <= UINT_MAX / digit );
-                const unsigned int digitScaled = mult * digit;
-                TIXMLASSERT( ucs <= ULONG_MAX - digitScaled );
-                ucs += digitScaled;
-                TIXMLASSERT( mult <= UINT_MAX / 16 );
-                mult *= 16;
-                --q;
-            }
+            q = p + 3;
         }
         else {
             // Decimal.
-            const char* q = p+2;
-            if ( !(*q) ) {
+            q = p + 2;
+        }
+        if (!(*q)) {
+            return 0;
+        }
+
+        q = strchr(q, SEMICOLON);
+        if (!q) {
+            return 0;
+        }
+        TIXMLASSERT(*q == SEMICOLON);
+
+        delta = q - p;
+        --q;
+
+        while (*q != terminator) {
+            uint32_t digit = 0;
+
+            if (*q >= '0' && *q <= '9') {
+                digit = *q - '0';
+            }
+            else if (hex && (*q >= 'a' && *q <= 'f')) {
+                digit = *q - 'a' + 10;
+            }
+            else if (hex && (*q >= 'A' && *q <= 'F')) {
+                digit = *q - 'A' + 10;
+            }
+            else {
                 return 0;
             }
+            TIXMLASSERT(digit < radix);
 
-            q = strchr( q, SEMICOLON );
-
-            if ( !q ) {
-                return 0;
+            const unsigned int digitScaled = mult * digit;
+            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.
+            if (mult > MAX_CODE_POINT) {
+                mult = MAX_CODE_POINT;
             }
-            TIXMLASSERT( *q == SEMICOLON );
-
-            delta = q-p;
             --q;
-
-            while ( *q != '#' ) {
-                if ( *q >= '0' && *q <= '9' ) {
-                    const unsigned int digit = *q - '0';
-                    TIXMLASSERT( digit < 10 );
-                    TIXMLASSERT( digit == 0 || mult <= UINT_MAX / digit );
-                    const unsigned int digitScaled = mult * digit;
-                    TIXMLASSERT( ucs <= ULONG_MAX - digitScaled );
-                    ucs += digitScaled;
-                }
-                else {
-                    return 0;
-                }
-                TIXMLASSERT( mult <= UINT_MAX / 10 );
-                mult *= 10;
-                --q;
-            }
+        }
+        // Out of range:
+        if (ucs > MAX_CODE_POINT) {
+            return 0;
         }
         // convert the UCS to UTF-8
-        ConvertUTF32ToUTF8( ucs, value, length );
+        ConvertUTF32ToUTF8(ucs, value, length);
         return p + delta + 1;
     }
-    return p+1;
+    return p + 1;
 }
 
-
 void XMLUtil::ToStr( int v, char* buffer, int bufferSize )
 {
     TIXML_SNPRINTF( buffer, bufferSize, "%d", v );
@@ -610,7 +592,7 @@ void XMLUtil::ToStr( int64_t v, char* buffer, int bufferSize )
 void XMLUtil::ToStr( uint64_t v, char* buffer, int bufferSize )
 {
     // horrible syntax trick to make the compiler happy about %llu
-    TIXML_SNPRINTF(buffer, bufferSize, "%llu", (long long)v);
+    TIXML_SNPRINTF(buffer, bufferSize, "%llu", static_cast<unsigned long long>(v));
 }
 
 bool XMLUtil::ToInt(const char* str, int* value)
@@ -705,7 +687,7 @@ bool XMLUtil::ToInt64(const char* str, int64_t* value)
 bool XMLUtil::ToUnsigned64(const char* str, uint64_t* value) {
     unsigned long long v = 0;	// horrible syntax trick to make the compiler happy about %llu
     if(TIXML_SSCANF(str, IsPrefixHex(str) ? "%llx" : "%llu", &v) == 1) {
-        *value = (uint64_t)v;
+        *value = static_cast<uint64_t>(v);
         return true;
     }
     return false;
@@ -1982,7 +1964,7 @@ char* XMLElement::ParseAttributes( char* p, int* curLineNumPtr )
         }
 
         // attribute.
-        if (XMLUtil::IsNameStartChar( (unsigned char) *p ) ) {
+        if (XMLUtil::IsNameStartChar( static_cast<unsigned char>(*p) ) ) {
             XMLAttribute* attrib = CreateAttribute();
             TIXMLASSERT( attrib );
             attrib->_parseLineNum = _document->_parseCurLineNum;
@@ -2226,7 +2208,7 @@ void XMLDocument::MarkInUse(const XMLNode* const node)
 	TIXMLASSERT(node);
 	TIXMLASSERT(node->_parent == 0);
 
-	for (int i = 0; i < _unlinked.Size(); ++i) {
+	for (size_t i = 0; i < _unlinked.Size(); ++i) {
 		if (node == _unlinked[i]) {
 			_unlinked.SwapRemove(i);
 			break;

tinyxml2.h

@@ -199,7 +199,7 @@ private:
 	Has a small initial memory pool, so that low or no usage will not
 	cause a call to new/delete
 */
-template <class T, int INITIAL_SIZE>
+template <class T, size_t INITIAL_SIZE>
 class DynArray
 {
 public:
@@ -227,9 +227,8 @@ public:
         ++_size;
     }
 
-    T* PushArr( int count ) {
-        TIXMLASSERT( count >= 0 );
-        TIXMLASSERT( _size <= INT_MAX - count );
+    T* PushArr( size_t count ) {
+        TIXMLASSERT( _size <= SIZE_MAX - count );
         EnsureCapacity( _size+count );
         T* ret = &_mem[_size];
         _size += count;
@@ -242,7 +241,7 @@ public:
         return _mem[_size];
     }
 
-    void PopArr( int count ) {
+    void PopArr( size_t count ) {
         TIXMLASSERT( _size >= count );
         _size -= count;
     }
@@ -251,13 +250,13 @@ public:
         return _size == 0;
     }
 
-    T& operator[](int i)				{
-        TIXMLASSERT( i>= 0 && i < _size );
+    T& operator[](size_t i) {
+        TIXMLASSERT( i < _size );
         return _mem[i];
     }
 
-    const T& operator[](int i) const	{
-        TIXMLASSERT( i>= 0 && i < _size );
+    const T& operator[](size_t i) const {
+        TIXMLASSERT( i < _size );
         return _mem[i];
     }
 
@@ -266,18 +265,18 @@ public:
         return _mem[ _size - 1];
     }
 
-    int Size() const					{
+    size_t Size() const {
         TIXMLASSERT( _size >= 0 );
         return _size;
     }
 
-    int Capacity() const				{
+    size_t Capacity() const {
         TIXMLASSERT( _allocated >= INITIAL_SIZE );
         return _allocated;
     }
 
-	void SwapRemove(int i) {
-		TIXMLASSERT(i >= 0 && i < _size);
+	void SwapRemove(size_t i) {
+		TIXMLASSERT(i < _size);
 		TIXMLASSERT(_size > 0);
 		_mem[i] = _mem[_size - 1];
 		--_size;
@@ -297,14 +296,14 @@ private:
     DynArray( const DynArray& ); // not supported
     void operator=( const DynArray& ); // not supported
 
-    void EnsureCapacity( int cap ) {
+    void EnsureCapacity( size_t cap ) {
         TIXMLASSERT( cap > 0 );
         if ( cap > _allocated ) {
-            TIXMLASSERT( cap <= INT_MAX / 2 );
-            const int newAllocated = cap * 2;
-            T* newMem = new T[static_cast<unsigned int>(newAllocated)];
+            TIXMLASSERT( cap <= SIZE_MAX / 2 / sizeof(T));
+            const size_t newAllocated = cap * 2;
+            T* newMem = new T[newAllocated];
             TIXMLASSERT( newAllocated >= _size );
-            memcpy( newMem, _mem, sizeof(T)*static_cast<size_t>(_size) );	// warning: not using constructors, only works for PODs
+            memcpy( newMem, _mem, sizeof(T) * _size );	// warning: not using constructors, only works for PODs
             if ( _mem != _pool ) {
                 delete [] _mem;
             }
@@ -314,9 +313,9 @@ private:
     }
 
     T*  _mem;
-    T   _pool[static_cast<size_t>(INITIAL_SIZE)];
-    int _allocated;		// objects allocated
-    int _size;			// number objects in use
+    T   _pool[INITIAL_SIZE];
+    size_t _allocated;		// objects allocated
+    size_t _size;			// number objects in use
 };
 
 
@@ -330,7 +329,7 @@ public:
     MemPool() {}
     virtual ~MemPool() {}
 
-    virtual int ItemSize() const = 0;
+    virtual size_t ItemSize() const = 0;
     virtual void* Alloc() = 0;
     virtual void Free( void* ) = 0;
     virtual void SetTracked() = 0;
@@ -340,7 +339,7 @@ public:
 /*
 	Template child class to create pools of the correct type.
 */
-template< int ITEM_SIZE >
+template< size_t ITEM_SIZE >
 class MemPoolT : public MemPool
 {
 public:
@@ -362,10 +361,10 @@ public:
         _nUntracked = 0;
     }
 
-    virtual int ItemSize() const override{
+    virtual size_t ItemSize() const override {
         return ITEM_SIZE;
     }
-    int CurrentAllocs() const		{
+    size_t CurrentAllocs() const {
         return _currentAllocs;
     }
 
@@ -376,7 +375,7 @@ public:
             _blockPtrs.Push( block );
 
             Item* blockItems = block->items;
-            for( int i = 0; i < ITEMS_PER_BLOCK - 1; ++i ) {
+            for( size_t i = 0; i < ITEMS_PER_BLOCK - 1; ++i ) {
                 blockItems[i].next = &(blockItems[i + 1]);
             }
             blockItems[ITEMS_PER_BLOCK - 1].next = 0;
@@ -417,7 +416,7 @@ public:
         --_nUntracked;
     }
 
-    int Untracked() const {
+    size_t Untracked() const {
         return _nUntracked;
     }
 
@@ -448,10 +447,10 @@ private:
     DynArray< Block*, 10 > _blockPtrs;
     Item* _root;
 
-    int _currentAllocs;
-    int _nAllocs;
-    int _maxAllocs;
-    int _nUntracked;
+    size_t _currentAllocs;
+    size_t _nAllocs;
+    size_t _maxAllocs;
+    size_t _nUntracked;
 };
 
 
@@ -892,7 +891,7 @@ public:
 
 		If the 'target' is null, then the nodes will
 		be allocated in the current document. If 'target'
-        is specified, the memory will be allocated is the
+        is specified, the memory will be allocated in the
         specified XMLDocument.
 
 		NOTE: This is probably not the correct tool to
@@ -1981,11 +1980,11 @@ private:
 	void PushDepth();
 	void PopDepth();
 
-    template<class NodeType, int PoolElementSize>
+    template<class NodeType, size_t PoolElementSize>
     NodeType* CreateUnlinkedNode( MemPoolT<PoolElementSize>& pool );
 };
 
-template<class NodeType, int PoolElementSize>
+template<class NodeType, size_t PoolElementSize>
 inline NodeType* XMLDocument::CreateUnlinkedNode( MemPoolT<PoolElementSize>& pool )
 {
     TIXMLASSERT( sizeof( NodeType ) == PoolElementSize );
@@ -2315,7 +2314,7 @@ public:
     	of the XML file in memory. (Note the size returned
     	includes the terminating null.)
     */
-    int CStrSize() const {
+    size_t CStrSize() const {
         return _buffer.Size();
     }
     /**
@@ -2375,7 +2374,7 @@ private:
 };
 
 
-}	// tinyxml2
+} // namespace tinyxml2
 
 #if defined(_MSC_VER)
 #   pragma warning(pop)

xmltest.cpp

@@ -1642,7 +1642,7 @@ int main( int argc, const char ** argv )
 
 		static const char* result  = "\xef\xbb\xbf<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
 		XMLTest( "BOM and default declaration", result, printer.CStr(), false );
-		XMLTest( "CStrSize", 42, printer.CStrSize(), false );
+		XMLTest( "CStrSize", true, printer.CStrSize() == 42, false );
 	}
 	{
 		const char* xml = "<ipxml ws='1'><info bla=' /></ipxml>";
@@ -2666,6 +2666,35 @@ int main( int argc, const char ** argv )
     	doc.PrintError();
     }
 
+	// ---------- CVE-2024-50615 -----------
+	{
+		const char* xml = "<Hello value='12&#65;34' value2='56&#x42;78'>Text</Hello>";
+		XMLDocument doc;
+		doc.Parse(xml);
+		const char* value = doc.FirstChildElement()->Attribute("value");
+		const char* value2 = doc.FirstChildElement()->Attribute("value2");
+		XMLTest("Test attribute encode", false, doc.Error());
+		XMLTest("Test decimal value", value, "12A34");
+		XMLTest("Test hex encode", value2, "56B78");
+	}
+
+	{
+		const char* xml = "<Hello value='&#ABC9000000065;' value2='&#xffffffff;' value3='&#5000000000;' value4='&#x00000045;' value5='&#x000000000000000021;'>Text</Hello>";
+		XMLDocument doc;
+		doc.Parse(xml);
+		const char* value = doc.FirstChildElement()->Attribute("value");
+		const char* value2 = doc.FirstChildElement()->Attribute("value2");
+		const char* value3 = doc.FirstChildElement()->Attribute("value3");
+		const char* value4 = doc.FirstChildElement()->Attribute("value4");
+		const char* value5 = doc.FirstChildElement()->Attribute("value5");
+		XMLTest("Test attribute encode", false, doc.Error());
+		XMLTest("Test attribute encode too long value", value, "&#ABC9000000065;"); // test long value
+		XMLTest("Test attribute encode out of unicode range", value2, "&#xffffffff;"); // out of unicode range
+		XMLTest("Test attribute encode out of int max value", value3, "&#5000000000;"); // out of int max value
+		XMLTest("Test attribute encode with a Hex value", value4, "E"); // hex value in unicode value
+		XMLTest("Test attribute encode with a Hex value", value5, "!"); // hex value in unicode value
+	}
+
     // ----------- Performance tracking --------------
 	{
 #if defined( _MSC_VER )