From e7d6f1849b5649d1fb903f11afcf88f4db5b3fb92e9261375c4155ad4c8d4eec Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Mon, 19 Aug 2024 09:59:07 +0200 Subject: [PATCH] Add crash fixes for >=1GB XML documents --- ...ter-support-DynArrays-larger-than-2G.patch | 52 ++++ ...pport-objects-larger-than-1-gigabyte.patch | 244 ++++++++++++++++++ tinyxml2.changes | 8 + tinyxml2.spec | 2 + 4 files changed, 306 insertions(+) create mode 100644 0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch create mode 100644 0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch diff --git a/0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch b/0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch new file mode 100644 index 0000000..965d3c2 --- /dev/null +++ b/0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch @@ -0,0 +1,52 @@ +From 04bbc06cd0d1fbbcebd91a8cd376a0d3c5b3cb27 Mon Sep 17 00:00:00 2001 +From: Jan Engelhardt +Date: Wed, 14 Aug 2024 15:19:05 +0200 +Subject: [PATCH] Make DocPrinter support DynArrays larger than 2G +References: https://github.com/leethomason/tinyxml2/pull/993 + +If the DynArray within an XMLPrinter object carries 2 gigabytes of +data or more, XMLPrinter::CStrSize returns a truncated result. If a +program casts this back to size_t without thought, sign extension +leads to bad things(tm). + +```c++ +int main() +{ + tinyxml2::XMLDocument doc; + doc.InsertEndChild(doc.NewDeclaration()); + auto root = doc.NewElement("root"); + size_t sz = 0x80000002; + auto blank = new char[sz]; + memset(blank, ' ', sz); + blank[sz-1]='\0'; + root->SetText(blank); + doc.InsertEndChild(root); + tinyxml2::XMLPrinter printer(nullptr); + doc.Print(&printer); + std::string_view sv{printer.CStr(), static_cast(printer.CStrSize())}; + // sv.size() is way too big, causing overflows on access + std::string dup(sv); // boom +} +``` + +Fixes: 2.0.2-873-geb3ab0d +--- + tinyxml2.h | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/tinyxml2.h b/tinyxml2.h +index d5a3afd..cdd6880 100644 +--- a/tinyxml2.h ++++ b/tinyxml2.h +@@ -2314,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(); + } + /** +-- +2.46.0 + diff --git a/0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch b/0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch new file mode 100644 index 0000000..56398a6 --- /dev/null +++ b/0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch @@ -0,0 +1,244 @@ +From eb3ab0df5d184b84eb283c0806f6abee71e3409b Mon Sep 17 00:00:00 2001 +From: Jan Engelhardt +Date: Mon, 1 Jul 2024 14:23:34 +0200 +Subject: [PATCH] Make DynArray support objects larger than 1 gigabyte +References: https://github.com/leethomason/tinyxml2/pull/990 + +The expression ``const int newAllocated = cap * 2;`` easily causes +overflow, as soon as the input is 1.0 GiB. This goes unnoticed because +release builds of tinyxml2 do not have active assertions. + +The change in commit 9.0.0-20-g8fd6cc6 did not do anything useful; +the signed multiplication overflow (and thus undefined behavior) +still occurs. + +Using ``int`` in this class is really archaic, because it limits the +class to a gigabyte even on 64-bit platforms. + +The multiplication overflow check also needs to include sizeof(T), +otherwise you can run into unsigned multiplication overflow (defined, +but undesirable) in the memcpy() call. + +testcase: + +int main() +{ + tinyxml2::XMLDocument doc; + doc.InsertEndChild(doc.NewDeclaration()); + auto root = doc.NewElement("root"); + size_t sz = 0x80000001; + auto blank = new char[sz]; + memset(blank, ' ', sz); + blank[sz-1]='\0'; + root->SetText(blank); + doc.InsertEndChild(root); + tinyxml2::XMLPrinter printer(nullptr); + doc.Print(&printer); +} +--- + tinyxml2.cpp | 2 +- + tinyxml2.h | 65 ++++++++++++++++++++++++++-------------------------- + 2 files changed, 33 insertions(+), 34 deletions(-) + +diff --git a/tinyxml2.cpp b/tinyxml2.cpp +index 6d6c200..0fed8dc 100644 +--- a/tinyxml2.cpp ++++ b/tinyxml2.cpp +@@ -2221,7 +2221,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; +diff --git a/tinyxml2.h b/tinyxml2.h +index 7586f7b..d5a3afd 100644 +--- a/tinyxml2.h ++++ b/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 ++template + 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(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) ); // 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(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; + }; + + +@@ -1981,11 +1980,11 @@ private: + void PushDepth(); + void PopDepth(); + +- template ++ template + NodeType* CreateUnlinkedNode( MemPoolT& pool ); + }; + +-template ++template + inline NodeType* XMLDocument::CreateUnlinkedNode( MemPoolT& pool ) + { + TIXMLASSERT( sizeof( NodeType ) == PoolElementSize ); +-- +2.45.2 + diff --git a/tinyxml2.changes b/tinyxml2.changes index 8ca08bd..f05cdac 100644 --- a/tinyxml2.changes +++ b/tinyxml2.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Aug 19 07:58:36 UTC 2024 - Jan Engelhardt + +- Add + 0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch + 0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch + to fix crashes when working with XML documents larger than 1 GB. + ------------------------------------------------------------------- Tue May 7 12:48:07 UTC 2024 - Jan Engelhardt diff --git a/tinyxml2.spec b/tinyxml2.spec index 27a9d73..a5370cb 100644 --- a/tinyxml2.spec +++ b/tinyxml2.spec @@ -26,6 +26,8 @@ License: Zlib Group: Development/Libraries/C and C++ URL: https://github.com/leethomason/tinyxml2 Source: https://github.com/leethomason/tinyxml2/archive/%{version}.tar.gz +Patch1: 0001-Make-DynArray-support-objects-larger-than-1-gigabyte.patch +Patch2: 0001-Make-DocPrinter-support-DynArrays-larger-than-2G.patch BuildRequires: cmake >= 3.15 BuildRequires: gcc-c++ BuildRequires: pkgconfig