[PATCH] Develogs.de: prevent undefined behaviour

Lubomir I. Ivanov neolit123 at gmail.com
Tue Dec 10 12:53:02 UTC 2013


From: "Lubomir I. Ivanov" <neolit123 at gmail.com>

prepare_dives_for_divelogs() did a silly thing, which I was
responsible for. When populating 'tempfile' we benefit
from QString, but then return a pointer to a local variable
(char *) without alocating it on the heap. This resulted
in undefined behavior, as we don't know the lifespan of that
local memory on the stack.

Patch fixes that by using strdup() and freeing the memory
when/if needed.

Suggested-by: Linus Torvalds <torvalds at linux-foundation.org>
Signed-off-by: Lubomir I. Ivanov <neolit123 at gmail.com>
---
 qt-ui/subsurfacewebservices.cpp | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
index a8421de..b974191 100644
--- a/qt-ui/subsurfacewebservices.cpp
+++ b/qt-ui/subsurfacewebservices.cpp
@@ -115,15 +115,17 @@ static char *prepare_dives_for_divelogs(const bool selected)
 
 	/* generate a random filename and create/open that file with zip_open */
 	QString tempfileQ = QDir::tempPath() + "/import-" + QString::number(qrand() % 99999999) + ".dld";
-	tempfile = tempfileQ.toLocal8Bit().data();
+	tempfile = strdup(tempfileQ.toLocal8Bit().data());
 	zip = zip_open(tempfile, ZIP_CREATE, NULL);
 
 	if (!zip) {
 		qDebug() << errPrefix << "cannot open file as zip";
+		free((void *)tempfile);
 		return NULL;
 	}
 	if (!amount_selected) {
 		qDebug() << errPrefix << "no dives selected";
+		free((void *)tempfile);
 		return NULL;
 	}
 
@@ -137,6 +139,7 @@ static char *prepare_dives_for_divelogs(const bool selected)
 		f = tmpfile();
 		if (!f) {
 			qDebug() << errPrefix << "cannot create temp file";
+			free((void *)tempfile);
 			return NULL;
 		}
 		save_dive(f, dive);
@@ -146,6 +149,7 @@ static char *prepare_dives_for_divelogs(const bool selected)
 		membuf = (char *)malloc(streamsize + 1);
 		if (!membuf || !fread(membuf, streamsize, 1, f)) {
 			qDebug() << errPrefix << "memory error";
+			free((void *)tempfile);
 			return NULL;
 		}
 		membuf[streamsize] = 0;
@@ -158,6 +162,7 @@ static char *prepare_dives_for_divelogs(const bool selected)
 		doc = xmlReadMemory(membuf, strlen(membuf), "divelog", NULL, 0);
 		if (!doc) {
 			qDebug() << errPrefix << "xml error";
+			free((void *)tempfile);
 			return NULL;
 		}
 		free((void *)membuf);
@@ -165,6 +170,7 @@ static char *prepare_dives_for_divelogs(const bool selected)
 		xslt = get_stylesheet("divelogs-export.xslt");
 		if (!xslt) {
 			qDebug() << errPrefix << "missing stylesheet";
+			free((void *)tempfile);
 			return NULL;
 		}
 		transformed = xsltApplyStylesheet(xslt, doc, NULL);
@@ -184,8 +190,6 @@ static char *prepare_dives_for_divelogs(const bool selected)
 		}
 	}
 	zip_close(zip);
-	/* let's call this again */
-	tempfile = tempfileQ.toLocal8Bit().data();
 	return tempfile;
 }
 
@@ -554,9 +558,11 @@ void DivelogsDeWebServices::prepareDivesForUpload()
 			uploadDives((QIODevice *)&f);
 			f.close();
 			f.remove();
+			free((void *)filename);
 			return;
 		}
 		mainWindow()->showError(errorText.append(": ").append(filename));
+		free((void *)filename);
 		return;
 	}
 	mainWindow()->showError(errorText.append("!"));
-- 
1.7.11.msysgit.0



More information about the subsurface mailing list