Fix recorder crash due to use after free on user object.

The recorder used a ClientUser object pointer attached to the RecordBuffer
to be able to access a users session id and username . If the user joined
a server, made a noise and immediately left the server before the
RecordBuffer from the noise was processed the referenced ClientUser was
already deleted which crashed the client.

This fix retools RecordInfo and RecordBuffer to carry the information
previously needed from the ClientUser so that inside of the recorder
thread no dependencies to the ClientUser exist.

src/mumble/VoiceRecorder.cpp

@@ -42,21 +42,22 @@
 #include "../Timer.h"
 
 VoiceRecorder::RecordBuffer::RecordBuffer(
-        const ClientUser *cu,
+        int userIndex,
         boost::shared_array<float> buffer,
         int samples,
         quint64 absoluteStartSample)
 
-    : cuUser(cu)
+    : recordInfoIndex(userIndex)
     , fBuffer(buffer)
     , iSamples(samples)
     , absoluteStartSample(absoluteStartSample) {
 
 	// Nothing
 }
 
-VoiceRecorder::RecordInfo::RecordInfo(quint64 lastWrittenAbsoluteSample)
-    : sf(NULL)
+VoiceRecorder::RecordInfo::RecordInfo(quint64 lastWrittenAbsoluteSample, const QString& userName)
+    : userName(userName)
+    , sf(NULL)
     , lastWrittenAbsoluteSample(lastWrittenAbsoluteSample) {
 }
 
@@ -121,17 +122,12 @@ QString VoiceRecorder::sanitizeFilenameOrPathComponent(const QString &str) const
 	return res;
 }
 
-QString VoiceRecorder::expandTemplateVariables(const QString &path, boost::shared_ptr<RecordBuffer> rb) const {
+QString VoiceRecorder::expandTemplateVariables(const QString &path, const QString& userName) const {
 	// Split path into components
 	QString res;
 	QStringList comp = path.split(QLatin1Char('/'));
 	Q_ASSERT(!comp.isEmpty());
 
-	QString username(QLatin1String("Mixdown"));
-	// In mixdown mode |cuUser| is always NULL.
-	if (rb->cuUser)
-		username = rb->cuUser->qsName;
-
 	// Create a readable representation of the start date.
 	QString date(qdtRecordingStart.date().toString(Qt::ISODate));
 	QString time(qdtRecordingStart.time().toString(QLatin1String("hh-mm-ss")));
@@ -150,7 +146,7 @@ QString VoiceRecorder::expandTemplateVariables(const QString &path, boost::share
 	//		%time		Inserts the current time
 	//		%host		Inserts the hostname
 	QHash<const QString, QString> vars;
-	vars.insert(QLatin1String("user"), username);
+	vars.insert(QLatin1String("user"), userName);
 	vars.insert(QLatin1String("date"), date);
 	vars.insert(QLatin1String("time"), time);
 	vars.insert(QLatin1String("host"), hostname);
@@ -197,6 +193,12 @@ QString VoiceRecorder::expandTemplateVariables(const QString &path, boost::share
 	return res;
 }
 
+int VoiceRecorder::indexForUser(const ClientUser *clientUser) const {
+	Q_ASSERT(!bMixDown || clientUser == NULL);
+
+	return (bMixDown) ? 0 : clientUser->uiSession;
+}
+
 void VoiceRecorder::run() {
 	Q_ASSERT(!bRecording);
 	Q_ASSERT(iSampleRate != 0);
@@ -272,15 +274,13 @@ void VoiceRecorder::run() {
 				QMutexLocker l(&qmBufferLock);
 				rb = qlRecordBuffer.takeFirst();
 			}
-
-			// Use 0 as the |index| if multi channel recording is disabled.
-			int index = bMixDown ? 0 : rb->cuUser->uiSession;
-			Q_ASSERT(qhRecordInfo.contains(index));
-
+
 			// Create the file for this RecordInfo instance if it's not yet open.
-			boost::shared_ptr<RecordInfo> ri = qhRecordInfo.value(index);
+
+			Q_ASSERT(qhRecordInfo.contains(rb->recordInfoIndex));
+			boost::shared_ptr<RecordInfo> ri = qhRecordInfo.value(rb->recordInfoIndex);
 			if (ri->sf == NULL) {
-				QString filename = expandTemplateVariables(qsFileName, rb);
+				QString filename = expandTemplateVariables(qsFileName, ri->userName);
 
 				// Try to find a unique filename.
 				{
@@ -320,8 +320,7 @@ void VoiceRecorder::run() {
 				}
 
 				// Store the username in the title attribute of the file (if supported by the format).
-				if (rb->cuUser)
-					sf_set_string(ri->sf, SF_STR_TITLE, qPrintable(rb->cuUser->qsName));
+				sf_set_string(ri->sf, SF_STR_TITLE, qPrintable(ri->userName));
 			}
 
 			// Calculate the difference between the time of the current buffer and the time where we last wrote audio data for that user.
@@ -357,21 +356,24 @@ void VoiceRecorder::stop() {
 	qwcSleep.wakeAll();
 }
 
-void VoiceRecorder::addBuffer(const ClientUser *cu, boost::shared_array<float> buffer, int samples, quint64 absoluteSampleCount) {
-	Q_ASSERT(!bMixDown || cu == NULL);
+void VoiceRecorder::addBuffer(const ClientUser *clientUser, boost::shared_array<float> buffer, int samples, quint64 absoluteSampleCount) {
+	Q_ASSERT(!bMixDown || clientUser == NULL);
 
 	// Create a new RecordInfo object if this is a new user.
-	int index = bMixDown ? 0 : cu->uiSession;
+	const int index = indexForUser(clientUser);
+
 	if (!qhRecordInfo.contains(index)) {
-		boost::shared_ptr<RecordInfo> ri = boost::make_shared<RecordInfo>(m_firstSampleAbsolute);
+		boost::shared_ptr<RecordInfo> ri = boost::make_shared<RecordInfo>(
+		            m_firstSampleAbsolute, bMixDown ? QLatin1String("Mixdown")
+		                                            : clientUser->qsName);
 
 		qhRecordInfo.insert(index, ri);
 	}
 
 	{
 		// Save the buffer in |qlRecordBuffer|.
 		QMutexLocker l(&qmBufferLock);
-		boost::shared_ptr<RecordBuffer> rb = boost::make_shared<RecordBuffer>(cu, buffer, samples, absoluteSampleCount);
+		boost::shared_ptr<RecordBuffer> rb = boost::make_shared<RecordBuffer>(index, buffer, samples, absoluteSampleCount);
 		qlRecordBuffer << rb;
 	}
 

src/mumble/VoiceRecorder.h

@@ -81,13 +81,13 @@ class VoiceRecorder : public QThread {
 		// Stores information about a recording buffer.
 		struct RecordBuffer {
 			// Constructs a new RecordBuffer object.
-			explicit RecordBuffer(const ClientUser *cu,
-				                  boost::shared_array<float> buffer,
-				                  int samples,
-				                  quint64 absoluteStartSample);
+			RecordBuffer(int recordInfoIndex,
+			             boost::shared_array<float> buffer,
+			             int samples,
+			             quint64 absoluteStartSample);
 
-			// The user to which this buffer belongs.
-			const ClientUser *cuUser;
+			/// Hashmap index for the user
+			const int recordInfoIndex;
 
 			// The buffer.
 			boost::shared_array<float> fBuffer;
@@ -101,14 +101,18 @@ class VoiceRecorder : public QThread {
 
 		// Keep the recording state for one user.
 		struct RecordInfo {
-			explicit RecordInfo(quint64 lastWrittenAbsoluteSample);
+			RecordInfo(quint64 lastWrittenAbsoluteSample, const QString& userName);
 			~RecordInfo();
 
-			// libsndfile's handle.
+			/// Name of the user being recorded
+			const QString userName;
+
+			/// libsndfile's handle.
 			SNDFILE *sf;
 
 			/// The last absolute sample we wrote for this users
 			quint64 lastWrittenAbsoluteSample;
+
 		};
 
 		// Hash which maps the |uiSession| of all users for which we have to keep a recording state to the corresponding RecordInfo object.
@@ -154,9 +158,12 @@ class VoiceRecorder : public QThread {
 		// Removes invalid characters in a path component.
 		QString sanitizeFilenameOrPathComponent(const QString &str) const;
 
-		// Expands the template variables in |path| using the information contained in |rb|.
-		QString expandTemplateVariables(const QString &path, boost::shared_ptr<RecordBuffer> rb) const;
+		/// Expands the template variables in |path| for the given |userName|.
+		QString expandTemplateVariables(const QString &path, const QString& userName) const;
 
+		/// Returns the RecordInfo hashmap index for the given user
+		int indexForUser(const ClientUser *clientUser) const;
+
 	public:
 		// Error enum
 		enum Error { Unspecified, CreateDirectoryFailed, CreateFileFailed, InvalidSampleRate };
@@ -173,7 +180,7 @@ class VoiceRecorder : public QThread {
 
 		/// Adds an audio buffer which contains |samples| audio samples to the recorder.
 		/// The audio data will be aligned using the given |absoluteSampleCount|
-		void addBuffer(const ClientUser *cu, boost::shared_array<float> buffer, int samples, quint64 absoluteSampleCount);
+		void addBuffer(const ClientUser *clientUser, boost::shared_array<float> buffer, int samples, quint64 absoluteSampleCount);
 
 		// Sets the sample rate of the recorder. The sample rate can't change while the recoder is active.
 		void setSampleRate(int sampleRate);