[xerces-c] branch master updated: XMLReader constructor: fix memory leak when refreshRawBuffer() throws

This is an automated email from the ASF dual-hosted git repository.

rleigh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/xerces-c.git


The following commit(s) were added to refs/heads/master by this push:
     new cf436ab  XMLReader constructor: fix memory leak when refreshRawBuffer() throws
     new caa6515  Merge pull request #30 from rouault/fix_ossfuzz_37529
cf436ab is described below

commit cf436abc181ab65824f6f51ae087e166dbdcd249
Author: Even Rouault <[email protected]>
AuthorDate: Mon Aug 23 21:39:48 2021 +0200

    XMLReader constructor: fix memory leak when refreshRawBuffer() throws
    
    Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=37529 on GDAL
    
    The backtrace of the exception that caused the memory leak was:
    ```
    Catchpoint 1 (exception thrown), 0x00007ffff5547672 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
    (gdb) bt
    0  0x00007ffff5547672 in __cxa_throw () from /lib/x86_64-linux-gnu/libstdc++.so.6
    1  0x00007ffff24447c4 in xercesc_4_0::PosixFileMgr::fileRead (this=<optimized out>, f=<optimized out>, byteCount=<optimized out>, buffer=<optimized out>, manager=0x5555556df730)
       at xercesc/util/FileManagers/PosixFileMgr.cpp:160
    2  0x00007ffff24e6ec2 in xercesc_4_0::XMLReader::refreshRawBuffer (this=0x5555557e49f8) at xercesc/internal/XMLReader.cpp:1891
    3  0x00007ffff24e70d4 in xercesc_4_0::XMLReader::XMLReader (this=0x5555557e49f8, pubId=<optimized out>, sysId=0x555555750920 u"/", streamToAdopt=0x55555574e838, from=<optimized out>,
       type=xercesc_4_0::XMLReader::Type_General, source=xercesc_4_0::XMLReader::Source_External, throwAtEnd=false, calculateSrcOfs=false, lowWaterMark=100, version=xercesc_4_0::XMLReader::XMLV1_0,
       manager=0x5555556df730) at xercesc/internal/XMLReader.cpp:130
    4  0x00007ffff24ced75 in xercesc_4_0::ReaderMgr::createReader (this=this@entry=0x5555557896d8, src=..., refFrom=refFrom@entry=xercesc_4_0::XMLReader::RefFrom_NonLiteral,
       type=type@entry=xercesc_4_0::XMLReader::Type_General, source=source@entry=xercesc_4_0::XMLReader::Source_External, calcSrcOfs=false, lowWaterMark=100) at ./xercesc/sax/InputSource.hpp:314
    5  0x00007ffff24cb0af in xercesc_4_0::IGXMLScanner::scanReset (this=0x555555789608, src=...) at xercesc/internal/IGXMLScanner2.cpp:1286
    6  0x00007ffff24c36e9 in xercesc_4_0::IGXMLScanner::scanDocument (this=0x555555789608, src=...) at xercesc/internal/IGXMLScanner.cpp:198
    7  0x00007ffff250abaf in xercesc_4_0::AbstractDOMParser::parse (this=0x7fffffffc2d0, source=...) at xercesc/parsers/AbstractDOMParser.cpp:545
    8  0x00007ffff24cbdbe in xercesc_4_0::IGXMLScanner::resolveSchemaGrammar (this=0x555555792f78, loc=0x5555557dd694 u"/", uri=0x555555737180 u"`", ignoreLoadSchema=<optimized out>)
       at xercesc/internal/IGXMLScanner2.cpp:1895
      0x00007ffff24cce7c in xercesc_4_0::IGXMLScanner::parseSchemaLocation (this=0x555555792f78, schemaLocationStr=<optimized out>, ignoreLoadSchema=false) at ./xercesc/framework/XMLBuffer.hpp:171
    10 0x00007ffff24cd182 in xercesc_4_0::IGXMLScanner::scanRawAttrListforNameSpaces (this=this@entry=0x555555792f78, attCount=attCount@entry=9) at xercesc/internal/IGXMLScanner2.cpp:1649
    11 0x00007ffff24c22cb in xercesc_4_0::IGXMLScanner::scanStartTagNS (this=0x555555792f78, gotData=@0x7fffffffc91f: true) at xercesc/internal/IGXMLScanner.cpp:2213
    12 0x00007ffff24c3522 in xercesc_4_0::IGXMLScanner::scanContent (this=0x555555792f78) at xercesc/internal/IGXMLScanner.cpp:890
    13 0x00007ffff24c3760 in xercesc_4_0::IGXMLScanner::scanDocument (this=0x555555792f78, src=...) at xercesc/internal/IGXMLScanner.cpp:217
    14 0x00007ffff25158e3 in xercesc_4_0::SAX2XMLReaderImpl::parse (this=0x555555731828, source=...) at xercesc/parsers/SAX2XMLReaderImpl.cpp:409
    ```
---
 src/xercesc/internal/ReaderMgr.cpp |  6 +++++
 src/xercesc/internal/XMLReader.cpp | 47 +++++++++++++++++++++++++++++++++-----
 src/xercesc/internal/XMLReader.hpp |  2 ++
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/src/xercesc/internal/ReaderMgr.cpp b/src/xercesc/internal/ReaderMgr.cpp
index 0d92fc9..4e59a4a 100644
--- a/src/xercesc/internal/ReaderMgr.cpp
+++ b/src/xercesc/internal/ReaderMgr.cpp
@@ -436,6 +436,12 @@ XMLReader* ReaderMgr::createReader( const   InputSource&        src
                 );
         }
     }
+    catch(const XMLPlatformUtilsException&)
+    {
+        streamJanitor.release();
+
+        throw;
+    }
     catch(const OutOfMemoryException&)
     {
         streamJanitor.release();
diff --git a/src/xercesc/internal/XMLReader.cpp b/src/xercesc/internal/XMLReader.cpp
index 1facb53..bf43886 100644
--- a/src/xercesc/internal/XMLReader.cpp
+++ b/src/xercesc/internal/XMLReader.cpp
@@ -124,8 +124,16 @@ XMLReader::XMLReader(const  XMLCh* const          pubId
 {
     setXMLVersion(version);
 
-    // Do an initial load of raw bytes
-    refreshRawBuffer();
+    try
+    {
+        // Do an initial load of raw bytes
+        refreshRawBuffer();
+    }
+    catch (const XMLPlatformUtilsException&)
+    {
+        cleanup();
+        throw;
+    }
 
     // Ask the transcoding service if it supports src offset info
     fSrcOfsSupported = XMLPlatformUtils::fgTransService->supportsSrcOfs();
@@ -207,8 +215,16 @@ XMLReader::XMLReader(const  XMLCh* const          pubId
 {
     setXMLVersion(version);
 
-    // Do an initial load of raw bytes
-    refreshRawBuffer();
+    try
+    {
+        // Do an initial load of raw bytes
+        refreshRawBuffer();
+    }
+    catch (const XMLPlatformUtilsException&)
+    {
+        cleanup();
+        throw;
+    }
 
     // Copy the encoding string to our member
     fEncodingStr = XMLString::replicate(encodingStr, fMemoryManager);
@@ -390,8 +406,16 @@ XMLReader::XMLReader(const  XMLCh* const          pubId
 {
     setXMLVersion(version);
 
-    // Do an initial load of raw bytes
-    refreshRawBuffer();
+    try
+    {
+        // Do an initial load of raw bytes
+        refreshRawBuffer();
+    }
+    catch (const XMLPlatformUtilsException&)
+    {
+        cleanup();
+        throw;
+    }
 
     // Ask the transcoding service if it supports src offset info
     fSrcOfsSupported = XMLPlatformUtils::fgTransService->supportsSrcOfs();
@@ -456,11 +480,22 @@ XMLReader::XMLReader(const  XMLCh* const          pubId
 
 XMLReader::~XMLReader()
 {
+    cleanup();
+}
+
+
+void XMLReader::cleanup()
+{
     fMemoryManager->deallocate(fEncodingStr);
+    fEncodingStr = NULL;
     fMemoryManager->deallocate(fPublicId);
+    fPublicId = NULL;
     fMemoryManager->deallocate(fSystemId);
+    fSystemId = NULL;
     delete fStream;
+    fStream = NULL;
     delete fTranscoder;
+    fTranscoder = NULL;
 }
 
 
diff --git a/src/xercesc/internal/XMLReader.hpp b/src/xercesc/internal/XMLReader.hpp
index adb66cc..2b0bc27 100644
--- a/src/xercesc/internal/XMLReader.hpp
+++ b/src/xercesc/internal/XMLReader.hpp
@@ -253,6 +253,8 @@ private:
     // -----------------------------------------------------------------------
     //  Private helper methods
     // -----------------------------------------------------------------------
+    void cleanup();
+
     void checkForSwapped();
 
     void doInitCharSizeChecks();