zynaddsubfx

commit 3192a4f2da5fdd9d85d8abf8653c91317b08541e
parent 0ef9e954fe36e62538c73b028922087223a25fdd
Author: Hans Petter Selasky <[email protected]>
Date:   Sat,  8 Oct 2016 13:11:32 +0200

Fix for use after free in XMLwrapper.

If loading of a new XML file fails, the node and root pointers
can still be set pointing to freed elements. This happened
sometimes in the lv2-ttl-generator during build under FreeBSD.

The reason is that some current code tries to enterbranch() even
if loadXMLfile() has failed, and then the a stale node
pointer is followed. For example in BankDb.cpp:

> 131	    xml.loadXMLfile(getCacheName());
> 132	    if(xml.enterbranch("bank-cache")) {

Signed-off-by: Hans Petter Selasky <[email protected]>

Diffstat:
M
src/Misc/XMLwrapper.cpp
 | 
21
++++++++++++++-------
M
src/Misc/XMLwrapper.h
 | 
5
+++++

2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/src/Misc/XMLwrapper.cpp b/src/Misc/XMLwrapper.cpp
@@ -125,10 +125,21 @@ XMLwrapper::XMLwrapper()
     endbranch();
 }
 
-XMLwrapper::~XMLwrapper()
+void
+XMLwrapper::cleanup(void)
 {
     if(tree)
         mxmlDelete(tree);
+
+    /* make sure freed memory is not referenced */
+    tree = 0;
+    node = 0;
+    root = 0;
+}
+
+XMLwrapper::~XMLwrapper()
+{
+    cleanup();
 }
 
 void XMLwrapper::setPadSynth(bool enabled)
@@ -297,9 +308,7 @@ const char *trimLeadingWhite(const char *c)
 
 int XMLwrapper::loadXMLfile(const string &filename)
 {
-    if(tree != NULL)
-        mxmlDelete(tree);
-    tree = NULL;
+    cleanup();
 
     const char *xmldata = doloadfile(filename);
     if(xmldata == NULL)
@@ -367,10 +376,8 @@ char *XMLwrapper::doloadfile(const string &filename) const
 
 bool XMLwrapper::putXMLdata(const char *xmldata)
 {
-    if(tree != NULL)
-        mxmlDelete(tree);
+    cleanup();
 
-    tree = NULL;
     if(xmldata == NULL)
         return false;
 
diff --git a/src/Misc/XMLwrapper.h b/src/Misc/XMLwrapper.h
@@ -258,6 +258,11 @@ class XMLwrapper
          */
         char *doloadfile(const std::string &filename) const;
 
+        /**
+         * Cleanup XML tree before loading new one.
+         */
+        void cleanup(void);
+
         mxml_node_t *tree; /**<all xml data*/
         mxml_node_t *root; /**<xml data used by zynaddsubfx*/
         mxml_node_t *node; /**<current subtree in parsing or writing */