[ 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