rouault commented on code in PR #51:
URL: https://github.com/apache/xerces-c/pull/51#discussion_r986039665
##
src/xercesc/validators/common/DFAContentModel.cpp:
##
@@ -661,8 +662,15 @@ void DFAContentModel::buildDFA(ContentSpecNode* const
curNode)
// in the fLeafCount member.
//
fLeafCount=countLeafNodes(curNode);
+// Avoid integer overflow in below fLeafCount++ increment
+if (fLeafCount > std::numeric_limits::max() - 1)
Review Comment:
fLeafCount is actually declared as unsigned int at line 236 of
DFAContentModel.hpp (which is already much bigger than what is reasonable in
practice :-))
##
src/xercesc/validators/common/DFAContentModel.cpp:
##
@@ -661,8 +662,15 @@ void DFAContentModel::buildDFA(ContentSpecNode* const
curNode)
// in the fLeafCount member.
//
fLeafCount=countLeafNodes(curNode);
+// Avoid integer overflow in below fLeafCount++ increment
+if (fLeafCount > std::numeric_limits::max() - 1)
+throw OutOfMemoryException();
fEOCPos = fLeafCount++;
+// Avoid integer overflow in below memory allocatoin
+if (fLeafCount > std::numeric_limits::max() / sizeof(CMLeaf*))
Review Comment:
> Would it be possible to add brackets around the division for clarity?
this is the best way I can think of that doesn't rely on doing overflow
arithmetic... As it is unsigned type, we could do overflow arithmetic as it is
well defined in C/C++, but I tend to fuzz software with
-fsanitize=unsigned-integer-overflow because in practice > 90% unsigned
overflows that occur are actually bugs
##
src/xercesc/validators/common/DFAContentModel.cpp:
##
@@ -661,8 +662,15 @@ void DFAContentModel::buildDFA(ContentSpecNode* const
curNode)
// in the fLeafCount member.
//
fLeafCount=countLeafNodes(curNode);
+// Avoid integer overflow in below fLeafCount++ increment
+if (fLeafCount > std::numeric_limits::max() - 1)
+throw OutOfMemoryException();
fEOCPos = fLeafCount++;
+// Avoid integer overflow in below memory allocatoin
Review Comment:
fixed
##
src/xercesc/validators/common/DFAContentModel.cpp:
##
@@ -1364,14 +1372,27 @@ unsigned int
DFAContentModel::countLeafNodes(ContentSpecNode* const curNode)
if(nLoopCount!=0)
{
count += countLeafNodes(cursor);
-for(unsigned int i=0;i std::numeric_limits::max() /
nLoopCount)
Review Comment:
cf above comments regarding data type
brackets added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org
