fix the potential overflow in char refs

leethomason · Dec 5, 2024 · 494735d · 494735d
1 parent d418ac2
commit 494735d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 14 deletions.
diff --git a/tinyxml2.cpp b/tinyxml2.cpp
@@ -472,11 +472,12 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
     // Presume an entity, and pull it out.
     *length = 0;
 
+	static const uint32_t MAX_CODE_POINT = 0x10FFFF;
+
     if ( *(p+1) == '#' && *(p+2) ) {
-        unsigned long ucs = 0;
-        TIXMLASSERT( sizeof( ucs ) >= 4 );
+        uint32_t ucs = 0;
         ptrdiff_t delta = 0;
-        unsigned mult = 1;
+        uint32_t mult = 1;
         static const char SEMICOLON = ';';
 
         if ( *(p+2) == 'x' ) {
@@ -487,7 +488,6 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
             }
 
             q = strchr( q, SEMICOLON );
-
             if ( !q ) {
                 return 0;
             }
@@ -497,7 +497,7 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
             --q;
 
             while ( *q != 'x' ) {
-                unsigned int digit = 0;
+                uint32_t digit = 0;
 
                 if ( *q >= '0' && *q <= '9' ) {
                     digit = *q - '0';
@@ -512,11 +512,12 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
                     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 );
+				if (ucs > MAX_CODE_POINT) {
+					return 0;
+				}
+
                 mult *= 16;
                 --q;
             }
@@ -540,22 +541,23 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
 
             while ( *q != '#' ) {
                 if ( *q >= '0' && *q <= '9' ) {
-                    const unsigned int digit = *q - '0';
+                    const uint32_t digit = *q - '0';
                     TIXMLASSERT( digit < 10 );
-                    TIXMLASSERT( digit == 0 || mult <= UINT_MAX / digit );
-                    const unsigned int digitScaled = mult * digit;
-                    TIXMLASSERT( ucs <= ULONG_MAX - digitScaled );
+                    const uint32_t digitScaled = mult * digit;
                     ucs += digitScaled;
+                    if (ucs > MAX_CODE_POINT) {
+                        return 0;
+                    }
                 }
                 else {
                     return 0;
                 }
-                TIXMLASSERT( mult <= UINT_MAX / 10 );
                 mult *= 10;
                 --q;
             }
         }
         // convert the UCS to UTF-8
+		TIXMLASSERT(ucs <= MAX_CODE_POINT);
         ConvertUTF32ToUTF8( ucs, value, length );
         return p + delta + 1;
     }

diff --git a/xmltest.cpp b/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 )