clean up the depth tracking a bit

This commit is contained in:
Lee Thomason
2018-04-05 09:24:20 -07:00
parent d946ddadc2
commit f928c35186
5 changed files with 24 additions and 16 deletions

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@@ -2384,21 +2384,18 @@ void XMLDocument::Parse()
ParseDeep(p, 0, &_parseCurLineNum ); ParseDeep(p, 0, &_parseCurLineNum );
} }
bool XMLDocument::PushDepth() void XMLDocument::PushDepth()
{ {
_parsingDepth++; _parsingDepth++;
if (_parsingDepth == TINYXML2_MAX_ELEMENT_DEPTH) { if (_parsingDepth == TINYXML2_MAX_ELEMENT_DEPTH) {
SetError(XMLError::XML_ELEMENT_DEPTH_EXCEEDED, _parseCurLineNum, "Element nesting is too deep." ); SetError(XMLError::XML_ELEMENT_DEPTH_EXCEEDED, _parseCurLineNum, "Element nesting is too deep." );
return false;
} }
return true;
} }
bool XMLDocument::PopDepth() void XMLDocument::PopDepth()
{ {
TIXMLASSERT(_parsingDepth > 0); TIXMLASSERT(_parsingDepth > 0);
--_parsingDepth; --_parsingDepth;
return true;
} }
XMLPrinter::XMLPrinter( FILE* file, bool compact, int depth ) : XMLPrinter::XMLPrinter( FILE* file, bool compact, int depth ) :

View File

@@ -106,10 +106,12 @@ static const int TIXML2_PATCH_VERSION = 0;
#define TINYXML2_MINOR_VERSION 1 #define TINYXML2_MINOR_VERSION 1
#define TINYXML2_PATCH_VERSION 0 #define TINYXML2_PATCH_VERSION 0
// This is problematic. There needs to be a limit to avoid a stack // A fixed element depth limit is problematic. There needs to be a
// overflow. However, that limit varies per system. Going with // limit to avoid a stack overflow. However, that limit varies per
// the MS value for now. May adjust in future versions. // system, and the capacity of the stack. On the other hand, it's a trivial
static const int TINYXML2_MAX_ELEMENT_DEPTH = 256; // attack that can result from ill, malicious, or even correctly formed XML,
// so there needs to be a limit in place.
static const int TINYXML2_MAX_ELEMENT_DEPTH = 100;
namespace tinyxml2 namespace tinyxml2
{ {
@@ -1915,8 +1917,8 @@ private:
private: private:
XMLDocument * _document; XMLDocument * _document;
}; };
bool PushDepth(); void PushDepth();
bool PopDepth(); void PopDepth();
template<class NodeType, int PoolElementSize> template<class NodeType, int PoolElementSize>
NodeType* CreateUnlinkedNode( MemPoolT<PoolElementSize>& pool ); NodeType* CreateUnlinkedNode( MemPoolT<PoolElementSize>& pool );

View File

@@ -2026,12 +2026,19 @@ int main( int argc, const char ** argv )
} }
{ {
// Bad bad crash. // Bad bad crash. Parsing error results in stack overflow, if uncaught.
XMLDocument doc; const char* TESTS[] = {
doc.LoadFile("./resources/xmltest-5330.xml"); "./resources/xmltest-5330.xml",
XMLTest("Stack overflow prevented.", XMLError::XML_ELEMENT_DEPTH_EXCEEDED, doc.ErrorID()); "./resources/xmltest-4636783552757760.xml",
"./resources/xmltest-5720541257269248.xml",
0
};
for (int i=0; TESTS[i]; ++i) {
XMLDocument doc;
doc.LoadFile(TESTS[i]);
XMLTest("Stack overflow prevented.", XMLError::XML_ELEMENT_DEPTH_EXCEEDED, doc.ErrorID());
}
} }
{ {
// Crashing reported via email. // Crashing reported via email.
const char* xml = const char* xml =