[
https://issues.apache.org/jira/browse/XERCESC-2188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17090335#comment-17090335
]
Andrew Williams commented on XERCESC-2188:
------------------------------------------
Could perhaps do more to clean-up in the event that the ReaderManager pops the
entity off the stack; but the following patch at least avoids the use-after
free without leaking.
{code:java}
diff -Naurw xerces-c-3.2.2.orig/src/xercesc/internal/DGXMLScanner.cpp
xerces-c-3.2.2/src/xercesc/internal/DGXMLScanner.cpp
--- xerces-c-3.2.2.orig/src/xercesc/internal/DGXMLScanner.cpp 2018-02-14
17:22:36.000000000 -0800
+++ xerces-c-3.2.2/src/xercesc/internal/DGXMLScanner.cpp 2020-04-22
20:42:09.426360000 -0700
@@ -65,6 +65,7 @@
, fElemCount(0)
, fAttDefRegistry(0)
, fUndeclaredAttrRegistry(0)
+ , fDTDEntityJanitor(4, true, manager)
{
CleanupType cleanup(this, &DGXMLScanner::cleanUp);
@@ -100,6 +101,7 @@
, fElemCount(0)
, fAttDefRegistry(0)
, fUndeclaredAttrRegistry(0)
+ , fDTDEntityJanitor(4, true, manager)
{
CleanupType cleanup(this, &DGXMLScanner::cleanUp);
@@ -1046,13 +1048,14 @@
// In order to make the processing work consistently, we have to
// make this look like an external entity. So create an entity
// decl and fill it in and push it with the reader, as happens
- // with an external entity. Put a janitor on it to insure it gets
- // cleaned up. The reader manager does not adopt them.
+ // with an external entity. Put a 'janitor' on it to ensure it
gets
+ // cleaned up. While the reader manager does not adopt them, it
may
+ // still dereference them.
const XMLCh gDTDStr[] = { chLatin_D, chLatin_T, chLatin_D , chNull
};
DTDEntityDecl* declDTD = new (fMemoryManager)
DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(sysId);
declDTD->setIsExternal(true);
- Janitor<DTDEntityDecl> janDecl(declDTD);
+ fDTDEntityJanitor.addElement(declDTD);
// Mark this one as a throw at end
reader->setThrowAtEnd(true);
@@ -2125,13 +2128,14 @@
// In order to make the processing work consistently, we have to
// make this look like an external entity. So create an entity
// decl and fill it in and push it with the reader, as happens
- // with an external entity. Put a janitor on it to insure it gets
- // cleaned up. The reader manager does not adopt them.
+ // with an external entity. Put a 'janitor' on it to ensure it gets
+ // cleaned up. While the reader manager does not adopt them, it may
+ // still dereference them.
const XMLCh gDTDStr[] = { chLatin_D, chLatin_T, chLatin_D , chNull };
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr,
false, fMemoryManager);
declDTD->setSystemId(src.getSystemId());
declDTD->setIsExternal(true);
- Janitor<DTDEntityDecl> janDecl(declDTD);
+ fDTDEntityJanitor.addElement(declDTD);
// Mark this one as a throw at end
newReader->setThrowAtEnd(true);
diff -Naurw xerces-c-3.2.2.orig/src/xercesc/internal/DGXMLScanner.hpp
xerces-c-3.2.2/src/xercesc/internal/DGXMLScanner.hpp
--- xerces-c-3.2.2.orig/src/xercesc/internal/DGXMLScanner.hpp 2018-02-14
17:22:36.000000000 -0800
+++ xerces-c-3.2.2/src/xercesc/internal/DGXMLScanner.hpp 2020-04-22
20:42:09.426360000 -0700
@@ -175,6 +175,8 @@
unsigned int fElemCount;
RefHashTableOf<unsigned int, PtrHasher>* fAttDefRegistry;
Hash2KeysSetOf<StringHasher>* fUndeclaredAttrRegistry;
+
+ RefVectorOf<DTDEntityDecl> fDTDEntityJanitor;
};
inline const XMLCh* DGXMLScanner::getName() const
diff -Naurw xerces-c-3.2.2.orig/src/xercesc/internal/IGXMLScanner.cpp
xerces-c-3.2.2/src/xercesc/internal/IGXMLScanner.cpp
--- xerces-c-3.2.2.orig/src/xercesc/internal/IGXMLScanner.cpp 2018-02-14
17:22:36.000000000 -0800
+++ xerces-c-3.2.2/src/xercesc/internal/IGXMLScanner.cpp 2020-04-22
20:42:09.426360000 -0700
@@ -85,6 +85,7 @@
, fErrorStack(0)
, fSchemaInfoList(0)
, fCachedSchemaInfoList (0)
+ , fDTDEntityJanitor(4, true, manager)
{
CleanupType cleanup(this, &IGXMLScanner::cleanUp);
@@ -138,6 +139,7 @@
, fErrorStack(0)
, fSchemaInfoList(0)
, fCachedSchemaInfoList (0)
+ , fDTDEntityJanitor(4, true, manager)
{
CleanupType cleanup(this, &IGXMLScanner::cleanUp);
@@ -1526,13 +1528,14 @@
// In order to make the processing work consistently, we have to
// make this look like an external entity. So create an entity
// decl and fill it in and push it with the reader, as happens
- // with an external entity. Put a janitor on it to insure it gets
- // cleaned up. The reader manager does not adopt them.
+ // with an external entity. Put a 'janitor' on it to ensure it
gets
+ // cleaned up. While the reader manager does not adopt them, it
may
+ // still dereference them.
const XMLCh gDTDStr[] = { chLatin_D, chLatin_T, chLatin_D , chNull
};
DTDEntityDecl* declDTD = new (fMemoryManager)
DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(sysId);
declDTD->setIsExternal(true);
- Janitor<DTDEntityDecl> janDecl(declDTD);
+ fDTDEntityJanitor.addElement(declDTD);
// Mark this one as a throw at end
reader->setThrowAtEnd(true);
@@ -3089,13 +3092,14 @@
// In order to make the processing work consistently, we have to
// make this look like an external entity. So create an entity
// decl and fill it in and push it with the reader, as happens
- // with an external entity. Put a janitor on it to insure it gets
- // cleaned up. The reader manager does not adopt them.
+ // with an external entity. Put a 'janitor' on it to ensure it gets
+ // cleaned up. While the reader manager does not adopt them, it may
+ // still dereference them.
const XMLCh gDTDStr[] = { chLatin_D, chLatin_T, chLatin_D , chNull };
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr,
false, fMemoryManager);
declDTD->setSystemId(src.getSystemId());
declDTD->setIsExternal(true);
- Janitor<DTDEntityDecl> janDecl(declDTD);
+ fDTDEntityJanitor.addElement(declDTD);
// Mark this one as a throw at end
newReader->setThrowAtEnd(true);
diff -Naurw xerces-c-3.2.2.orig/src/xercesc/internal/IGXMLScanner.hpp
xerces-c-3.2.2/src/xercesc/internal/IGXMLScanner.hpp
--- xerces-c-3.2.2.orig/src/xercesc/internal/IGXMLScanner.hpp 2018-02-14
17:22:36.000000000 -0800
+++ xerces-c-3.2.2/src/xercesc/internal/IGXMLScanner.hpp 2020-04-22
20:42:09.426360000 -0700
@@ -286,6 +286,8 @@
PSVIElemContext fPSVIElemContext;
RefHash2KeysTableOf<SchemaInfo>* fSchemaInfoList;
RefHash2KeysTableOf<SchemaInfo>* fCachedSchemaInfoList;
+
+ RefVectorOf<DTDEntityDecl> fDTDEntityJanitor;
};
inline const XMLCh* IGXMLScanner::getName() const
{code}
> Use-after-free on external DTD scan
> -----------------------------------
>
> Key: XERCESC-2188
> URL: https://issues.apache.org/jira/browse/XERCESC-2188
> Project: Xerces-C++
> Issue Type: Bug
> Components: Validating Parser (DTD)
> Affects Versions: 3.0.0, 3.0.1, 3.0.2, 3.1.0, 3.1.1, 3.1.2, 3.2.0, 3.1.3,
> 3.1.4, 3.2.1, 3.2.2
> Reporter: Scott Cantor
> Priority: Major
> Attachments: Apache-496067-disclosure-report.pdf
>
>
> This is a record of an unfixed bug reported in 2018 in the DTD scanner, per
> the attached PDF, corresponding to CVE-2018-1311.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org
