Avoid using free'd memory with QMailMessage::toRfc2822Chunks()HEADmaster

Executed scan-build which reported some problems in qmailmessage.cpp.
Worst one being referencing free'd memory.

QMailMessage::toRfc2822Chunks() creates a ChunkStore and then calls
partContainerImpl()->toRfc2822() with a pointer to ChunkStore's internal
QDataStream and functor that recreates the internal QDataStream.
Eventually the functor gets called in QMailMessagePartPrivate::output()
and the then output handling proceeds happily with the original pointer.

I suppose this should be fine by not creating a new instance at all.
Just clearing the target QByteArray should keep the chunks separate.

Also removed one case where variable 'malformed' was written with
no chance of getting the value read. Effectively dead code.
And made sure private partAt() methods don't refer nullptr
if nothing found.

Change-Id: Ia79e0d5087012666f08b6a5dd56c49c2bd4455f4
Reviewed-by: Pekka Vuorela <pvuorela@iki.fi>
Reviewed-by: <matti.viljanen@kapsi.fi>
Reviewed-by: Damien Caliste <dcaliste@free.fr>

1 files changed, 18 insertions, 17 deletions

diff --git a/src/libraries/qmfclient/qmailmessage.cpp b/src/libraries/qmfclient/qmailmessage.cpp
index 5ec19014..0089244a 100644
--- a/src/libraries/qmfclient/qmailmessage.cpp
+++ b/src/libraries/qmfclient/qmailmessage.cpp
@@ -1689,8 +1689,7 @@ void QMailMessageHeaderFieldPrivate::parse(const QByteArray& text, bool structur
             if (_id.isEmpty()) {
                 _id = QByteArray(token, (it - token)).trimmed();
                 token = (it + 1);
-            }
-            else if (_structured) {
+            } else if (_structured) {
                 // If this is a structured header, there can be only one colon
                 token = (it + 1);
             }
@@ -1733,8 +1732,6 @@ void QMailMessageHeaderFieldPrivate::parse(const QByteArray& text, bool structur
 
             if (!name.isEmpty() && !value.isEmpty())
                 addParameter(name, value);
-        } else if (_structured) {
-            malformed = true;
         }
     }
 }
@@ -4012,6 +4009,12 @@ const QMailMessagePart& QMailMessagePartContainerPrivate::partAt(const QMailMess
     }
 
     Q_ASSERT(part);
+    if (!part) {
+        qCWarning(lcMessaging) << Q_FUNC_INFO << "No QMailMessagePart found, this shouldn't happen";
+        static QMailMessagePart errorPart;
+        return errorPart;
+    }
+
     return *part;
 }
 
@@ -4030,6 +4033,12 @@ QMailMessagePart& QMailMessagePartContainerPrivate::partAt(const QMailMessagePar
         }
     }
 
+    if (!part) {
+        qCWarning(lcMessaging) << Q_FUNC_INFO << "No QMailMessagePart found, this shouldn't happen";
+        static QMailMessagePart errorPart;
+        return errorPart;
+    }
+
     return *part;
 }
 
@@ -8509,10 +8518,10 @@ struct ChunkStore
 {
     QList<QMailMessage::MessageChunk> chunks;
     QByteArray chunk;
-    QDataStream *ds;
+    QDataStream ds;
 
     ChunkStore()
-        : ds(new QDataStream(&chunk, QIODevice::WriteOnly | QIODevice::Unbuffered))
+        : ds(&chunk, QIODevice::WriteOnly | QIODevice::Unbuffered)
     {
     }
 
@@ -8523,24 +8532,16 @@ struct ChunkStore
 
     void close()
     {
-        if (ds) {
-            delete ds;
-            ds = nullptr;
-
-            if (!chunk.isEmpty()) {
-                chunks.append(qMakePair(QMailMessage::Text, chunk));
-            }
+        if (!chunk.isEmpty()) {
+            chunks.append(qMakePair(QMailMessage::Text, chunk));
         }
     }
 
     void operator()(QMailMessage::ChunkType type)
     {
         // This chunk is now complete
-        delete ds;
         chunks.append(qMakePair(type, chunk));
-
         chunk.clear();
-        ds = new QDataStream(&chunk, QIODevice::WriteOnly | QIODevice::Unbuffered);
     }
 };
 
@@ -8553,7 +8554,7 @@ QList<QMailMessage::MessageChunk> QMailMessage::toRfc2822Chunks(EncodingFormat f
 {
     ChunkStore store;
 
-    partContainerImpl()->toRfc2822<ChunkStore>(store.ds, format, status(), &store);
+    partContainerImpl()->toRfc2822<ChunkStore>(&store.ds, format, status(), &store);
     store.close();
 
     return store.chunks;