Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CVE-2024-50615 #1003

Closed
wants to merge 2 commits into from
Closed

Fix CVE-2024-50615 #1003

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b8ea04
Fix CVE-2024-50615
vanduc-nguyen Nov 1, 2024
016bd05
Allow any number of leading zero in Character Reference
vanduc-nguyen Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions tinyxml2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,10 +473,10 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
*length = 0;

if ( *(p+1) == '#' && *(p+2) ) {
unsigned long ucs = 0;
unsigned long long ucs = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be stdint types, not long long. Also - why the change? How is overflow being addressed?

TIXMLASSERT( sizeof( ucs ) >= 4 );
ptrdiff_t delta = 0;
unsigned mult = 1;
unsigned long long mult = 1;
static const char SEMICOLON = ';';

if ( *(p+2) == 'x' ) {
Expand All @@ -494,6 +494,11 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
TIXMLASSERT( *q == SEMICOLON );

delta = q-p;

if ( delta > 8 + 3 ) { // allow maximum 8 digits in hex format and '&#x'
return 0;
}

--q;

while ( *q != 'x' ) {
Expand All @@ -512,11 +517,8 @@ 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 );
const unsigned long long digitScaled = mult * digit;
ucs += digitScaled;
TIXMLASSERT( mult <= UINT_MAX / 16 );
mult *= 16;
--q;
}
Expand All @@ -536,27 +538,36 @@ const char* XMLUtil::GetCharacterRef( const char* p, char* value, int* length )
TIXMLASSERT( *q == SEMICOLON );

delta = q-p;

if (delta > 10 + 2) { // allow maximum 10 digits and '&#'
return 0;
}

--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 );
const unsigned long long digitScaled = mult * digit;
ucs += digitScaled;
}
else {
return 0;
}
TIXMLASSERT( mult <= UINT_MAX / 10 );
mult *= 10;
--q;
}
}

if (ucs > ULONG_MAX)
return 0;

// convert the UCS to UTF-8
ConvertUTF32ToUTF8( ucs, value, length );
ConvertUTF32ToUTF8( (unsigned long)ucs, value, length );
if (*length == 0)
return 0;

return p + delta + 1;
}
return p+1;
Expand Down
26 changes: 26 additions & 0 deletions xmltest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2666,6 +2666,32 @@ int main( int argc, const char ** argv )
doc.PrintError();
}

{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice tests

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;'>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");
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
}

// ----------- Performance tracking --------------
{
#if defined( _MSC_VER )
Expand Down