Skip to content

Commit

Permalink
XERCESC-2188 - Use-after-free on external DTD scan (CVE-2018-1311)
Browse files Browse the repository at this point in the history
These are the instructions for observing the bug (before this commit):

$ git clone https://github.com/apache/xerces-c.git
$ cd xerces-c
$ mkdir build
$ cd build
$ cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Debug ..
$ make -j8
$ cp ../samples/data/personal.xml .

$ cat <<EOF >personal.dtd
<?xml encoding="ISO-8859-1"?>
<!ENTITY % nonExistentEntity SYSTEM "non-existent.ent">
%nonExistentEntity;
EOF

$ gdb samples/StdInParse
(gdb) b IGXMLScanner.cpp:1544
(gdb) run <personal.xml
1544	            fReaderMgr.pushReader(reader, declDTD);
(gdb) p declDTD
$1 = (xercesc_4_0::DTDEntityDecl *) 0x49ac68
(gdb) n
1547	            dtdScanner.scanExtSubsetDecl(false, true);
(gdb) n
1548	        }
(gdb) s
...
(gdb) s                     # The Janitor is about to delete the above declDTD.
90	        delete fData;
(gdb) p fData
$1 = (xercesc_4_0::DTDEntityDecl *) 0x49ac68
(gdb) b ReaderMgr.cpp:1024
(gdb) n
...
(gdb) n                     # Now we about to dereference the deleted declDTD.
1024	    if (curEntity && !curEntity->isExternal())
(gdb) p curEntity
$2 = (const xercesc_4_0::XMLEntityDecl *) 0x49ac68
  • Loading branch information
karen-arutyunov authored and boris-kolpackov committed Dec 13, 2023
1 parent 5b31900 commit b38ab79
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 82 deletions.
6 changes: 2 additions & 4 deletions src/xercesc/internal/DGXMLScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1052,13 +1052,12 @@ void DGXMLScanner::scanDocTypeDecl()
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(sysId);
declDTD->setIsExternal(true);
Janitor<DTDEntityDecl> janDecl(declDTD);

// Mark this one as a throw at end
reader->setThrowAtEnd(true);

// And push it onto the stack, with its pseudo name
fReaderMgr.pushReader(reader, declDTD);
fReaderMgr.pushReaderAdoptEntity(reader, declDTD);

// Tell it its not in an include section
dtdScanner.scanExtSubsetDecl(false, true);
Expand Down Expand Up @@ -2131,13 +2130,12 @@ Grammar* DGXMLScanner::loadDTDGrammar(const InputSource& src,
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(src.getSystemId());
declDTD->setIsExternal(true);
Janitor<DTDEntityDecl> janDecl(declDTD);

// Mark this one as a throw at end
newReader->setThrowAtEnd(true);

// And push it onto the stack, with its pseudo name
fReaderMgr.pushReader(newReader, declDTD);
fReaderMgr.pushReaderAdoptEntity(newReader, declDTD);

// If we have a doc type handler and advanced callbacks are enabled,
// call the doctype event.
Expand Down
6 changes: 2 additions & 4 deletions src/xercesc/internal/IGXMLScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,13 +1535,12 @@ void IGXMLScanner::scanDocTypeDecl()
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(sysId);
declDTD->setIsExternal(true);
Janitor<DTDEntityDecl> janDecl(declDTD);

// Mark this one as a throw at end
reader->setThrowAtEnd(true);

// And push it onto the stack, with its pseudo name
fReaderMgr.pushReader(reader, declDTD);
fReaderMgr.pushReaderAdoptEntity(reader, declDTD);

// Tell it its not in an include section
dtdScanner.scanExtSubsetDecl(false, true);
Expand Down Expand Up @@ -3098,13 +3097,12 @@ Grammar* IGXMLScanner::loadDTDGrammar(const InputSource& src,
DTDEntityDecl* declDTD = new (fMemoryManager) DTDEntityDecl(gDTDStr, false, fMemoryManager);
declDTD->setSystemId(src.getSystemId());
declDTD->setIsExternal(true);
Janitor<DTDEntityDecl> janDecl(declDTD);

// Mark this one as a throw at end
newReader->setThrowAtEnd(true);

// And push it onto the stack, with its pseudo name
fReaderMgr.pushReader(newReader, declDTD);
fReaderMgr.pushReaderAdoptEntity(newReader, declDTD);

// If we have a doc type handler and advanced callbacks are enabled,
// call the doctype event.
Expand Down
Loading

0 comments on commit b38ab79

Please sign in to comment.