Bug 1333038 - Use 'modern' pointers to fix crash due to nsMsgLineStreamBuffer object being deleted while still in use.

Suspected "use after free" in nsMsgLineStreamBuffer::ReadNextLine() leading to crash since object may be destroyed while still in use on another thread.

Tag #1273

13 files changed, 14 insertions, 26 deletions

diff --git a/mailnews/base/util/nsMsgLineBuffer.h b/mailnews/base/util/nsMsgLineBuffer.h
index eff3f7c7ee..6bacd45af9 100644
--- a/mailnews/base/util/nsMsgLineBuffer.h
+++ b/mailnews/base/util/nsMsgLineBuffer.h
@@ -70,6 +70,8 @@ class nsIInputStream;
 class NS_MSG_BASE nsMsgLineStreamBuffer
 {
 public:
+  NS_INLINE_DECL_REFCOUNTING(nsMsgLineStreamBuffer)
+
   // aBufferSize -- size of the buffer you want us to use for buffering stream data
   // aEndOfLinetoken -- The delimiter string to be used for determining the end of line. This 
   //              allows us to parse platform specific end of line endings by making it
@@ -83,7 +85,6 @@ public:
   //            lines are terminated with a CR only, you need to set aLineToken to CR ('\r')
   nsMsgLineStreamBuffer(uint32_t aBufferSize, bool aAllocateNewLines, 
                         bool aEatCRLFs = true, char aLineToken = '\n'); // specify the size of the buffer you want the class to use....
-  virtual ~nsMsgLineStreamBuffer();
   
   // Caller must free the line returned using PR_Free
   // aEndOfLinetoken -- delimiter used to denote the end of a line.
@@ -93,6 +94,8 @@ public:
   nsresult GrowBuffer(int32_t desiredSize);
   void ClearBuffer();
   bool NextLineAvailable();
+private:
+  virtual ~nsMsgLineStreamBuffer();
 protected:
   bool m_eatCRLFs;
   bool m_allocateNewLines;
diff --git a/mailnews/compose/src/nsSmtpProtocol.cpp b/mailnews/compose/src/nsSmtpProtocol.cpp
index 04347c1d05..522890c789 100644
--- a/mailnews/compose/src/nsSmtpProtocol.cpp
+++ b/mailnews/compose/src/nsSmtpProtocol.cpp
@@ -236,7 +236,6 @@ nsSmtpProtocol::~nsSmtpProtocol()
 {
   // free our local state
   PR_Free(m_dataBuf);
-  delete m_lineStreamBuffer;
 }
 
 void nsSmtpProtocol::Initialize(nsIURI * aURL)
diff --git a/mailnews/compose/src/nsSmtpProtocol.h b/mailnews/compose/src/nsSmtpProtocol.h
index c8cbf1406b..b1370d4be0 100644
--- a/mailnews/compose/src/nsSmtpProtocol.h
+++ b/mailnews/compose/src/nsSmtpProtocol.h
@@ -143,7 +143,7 @@ private:
     int32_t m_previousResponseCode; 
     int32_t m_continuationResponse;
     nsCString m_responseText;   /* text returned from Smtp server */
-    nsMsgLineStreamBuffer *m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
+    RefPtr<nsMsgLineStreamBuffer> m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
 
     nsTArray<nsCString> m_addresses;
     uint32_t       m_addressesLeft;
diff --git a/mailnews/imap/src/nsImapMailFolder.cpp b/mailnews/imap/src/nsImapMailFolder.cpp
index 4fade9d3fe..da1411cd08 100644
--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -8379,7 +8379,7 @@ nsImapMailFolder::CopyFileToOfflineStore(nsIFile *srcFile, nsMsgKey msgKey)
   {
     // Now, parse the temp file to (optionally) copy to
     // the offline store for the cur folder.
-    nsMsgLineStreamBuffer *inputStreamBuffer =
+    RefPtr<nsMsgLineStreamBuffer> inputStreamBuffer =
       new nsMsgLineStreamBuffer(FILE_IO_BUFFER_SIZE, true, false);
     int64_t fileSize;
     srcFile->GetFileSize(&fileSize);
@@ -8443,7 +8443,6 @@ nsImapMailFolder::CopyFileToOfflineStore(nsIFile *srcFile, nsMsgKey msgKey)
       notifier->NotifyMsgsClassified(messages, false, false);
     inputStream->Close();
     inputStream = nullptr;
-    delete inputStreamBuffer;
   }
   if (offlineStore)
     offlineStore->Close();
diff --git a/mailnews/imap/src/nsImapProtocol.cpp b/mailnews/imap/src/nsImapProtocol.cpp
index 5e2639a5a4..20cadc25cf 100644
--- a/mailnews/imap/src/nsImapProtocol.cpp
+++ b/mailnews/imap/src/nsImapProtocol.cpp
@@ -586,7 +586,6 @@ nsImapProtocol::~nsImapProtocol()
   NS_IF_RELEASE(m_flagState);
 
   PR_Free(m_dataOutputBuf);
-  delete m_inputStreamBuffer;
 
   // **** We must be out of the thread main loop function
   NS_ASSERTION(!m_imapThreadIsRunning, "Oops, thread is still running.\n");
diff --git a/mailnews/imap/src/nsImapProtocol.h b/mailnews/imap/src/nsImapProtocol.h
index 5c4f43abdf..32cf90e4c9 100644
--- a/mailnews/imap/src/nsImapProtocol.h
+++ b/mailnews/imap/src/nsImapProtocol.h
@@ -323,7 +323,7 @@ private:
   nsCString             m_serverKey;
   nsCString             m_realHostName;
   char                  *m_dataOutputBuf;
-  nsMsgLineStreamBuffer * m_inputStreamBuffer;
+  RefPtr<nsMsgLineStreamBuffer> m_inputStreamBuffer;
   uint32_t              m_allocatedSize; // allocated size
   uint32_t        m_totalDataSize; // total data size
   uint32_t        m_curReadIndex;  // current read index
diff --git a/mailnews/imap/src/nsImapService.cpp b/mailnews/imap/src/nsImapService.cpp
index 5e097311e1..1d97dec29d 100644
--- a/mailnews/imap/src/nsImapService.cpp
+++ b/mailnews/imap/src/nsImapService.cpp
@@ -2061,9 +2061,10 @@ nsresult nsImapService::OfflineAppendFromFile(nsIFile *aFile,
         if (NS_SUCCEEDED(rv) && inputStream)
         {
           // now, copy the temp file to the offline store for the dest folder.
-          nsMsgLineStreamBuffer *inputStreamBuffer = new nsMsgLineStreamBuffer(FILE_IO_BUFFER_SIZE,
-                                                                               true,    // allocate new lines
-                                                                               false);  // leave CRLFs on the returned string
+          RefPtr<nsMsgLineStreamBuffer> inputStreamBuffer =
+            new nsMsgLineStreamBuffer(FILE_IO_BUFFER_SIZE,
+                                      true,    // allocate new lines
+                                      false);  // leave CRLFs on the returned string
           int64_t fileSize;
           aFile->GetFileSize(&fileSize);
           uint32_t bytesWritten;
@@ -2109,7 +2110,6 @@ nsresult nsImapService::OfflineAppendFromFile(nsIFile *aFile,
           inputStream->Close();
           inputStream = nullptr;
           aListener->OnStopRunningUrl(aUrl, NS_OK);
-          delete inputStreamBuffer;
         }
         offlineStore->Close();
       }
diff --git a/mailnews/local/src/nsMailboxProtocol.cpp b/mailnews/local/src/nsMailboxProtocol.cpp
index fad77aa933..0148133b32 100644
--- a/mailnews/local/src/nsMailboxProtocol.cpp
+++ b/mailnews/local/src/nsMailboxProtocol.cpp
@@ -50,8 +50,6 @@ PRLogModuleInfo *MAILBOX;
 nsMailboxProtocol::nsMailboxProtocol(nsIURI * aURI)
     : nsMsgProtocol(aURI)
 {
-  m_lineStreamBuffer =nullptr;
-
   // initialize the pr log if it hasn't been initialiezed already
   if (!MAILBOX)
     MAILBOX = PR_NewLogModule("MAILBOX");
@@ -59,8 +57,6 @@ nsMailboxProtocol::nsMailboxProtocol(nsIURI * aURI)
 
 nsMailboxProtocol::~nsMailboxProtocol()
 {
-  // free our local state
-  delete m_lineStreamBuffer;
 }
 
 nsresult nsMailboxProtocol::OpenMultipleMsgTransport(uint64_t offset, int32_t size)
diff --git a/mailnews/local/src/nsMailboxProtocol.h b/mailnews/local/src/nsMailboxProtocol.h
index e030c8d7c7..36c8920696 100644
--- a/mailnews/local/src/nsMailboxProtocol.h
+++ b/mailnews/local/src/nsMailboxProtocol.h
@@ -74,7 +74,7 @@ private:
   nsCOMPtr<nsIStreamListener> m_mailboxParser;
 
   // Local state for the current operation
-  nsMsgLineStreamBuffer   * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
+  RefPtr<nsMsgLineStreamBuffer> m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
 
   // Generic state information -- What state are we in? What state do we want to go to
   // after the next response? What was the last response code? etc.
diff --git a/mailnews/local/src/nsPop3Protocol.cpp b/mailnews/local/src/nsPop3Protocol.cpp
index 825f45ab3d..5d9d9145a0 100644
--- a/mailnews/local/src/nsPop3Protocol.cpp
+++ b/mailnews/local/src/nsPop3Protocol.cpp
@@ -451,7 +451,6 @@ nsPop3Protocol::nsPop3Protocol(nsIURI* aURL)
   m_totalFolderSize(0),
   m_totalDownloadSize(0),
   m_totalBytesReceived(0),
-  m_lineStreamBuffer(nullptr),
   m_pop3ConData(nullptr)
 {
 }
@@ -590,9 +589,6 @@ void nsPop3Protocol::Cleanup()
   FreeMsgInfo();
   PR_Free(m_pop3ConData->only_uidl);
   PR_Free(m_pop3ConData);
-
-  delete m_lineStreamBuffer;
-  m_lineStreamBuffer = nullptr;
 }
 
 void nsPop3Protocol::SetCapFlag(uint32_t flag)
diff --git a/mailnews/local/src/nsPop3Protocol.h b/mailnews/local/src/nsPop3Protocol.h
index a332eae842..43a8aa4e83 100644
--- a/mailnews/local/src/nsPop3Protocol.h
+++ b/mailnews/local/src/nsPop3Protocol.h
@@ -318,7 +318,7 @@ private:
   nsCOMPtr<nsIPop3Sink> m_nsIPop3Sink;
   nsCOMPtr<nsIPop3IncomingServer> m_pop3Server;
 
-  nsMsgLineStreamBuffer   * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
+  RefPtr<nsMsgLineStreamBuffer> m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
   Pop3ConData* m_pop3ConData;
   void FreeMsgInfo();
   void Abort();
diff --git a/mailnews/news/src/nsNNTPProtocol.cpp b/mailnews/news/src/nsNNTPProtocol.cpp
index 7c6314d523..f721e9747f 100644
--- a/mailnews/news/src/nsNNTPProtocol.cpp
+++ b/mailnews/news/src/nsNNTPProtocol.cpp
@@ -271,7 +271,6 @@ nsNNTPProtocol::nsNNTPProtocol(nsINntpIncomingServer *aServer, nsIURI *aURL,
     NNTP = PR_NewLogModule("NNTP");
 
   m_ProxyServer = nullptr;
-  m_lineStreamBuffer = nullptr;
   m_responseText = nullptr;
   m_dataBuf = nullptr;
 
@@ -305,9 +304,6 @@ nsNNTPProtocol::~nsNNTPProtocol()
     m_nntpServer->WriteNewsrcFile();
     m_nntpServer->RemoveConnection(this);
   }
-  if (m_lineStreamBuffer) {
-     delete m_lineStreamBuffer;
-  }
   if (mUpdateTimer) {
     mUpdateTimer->Cancel();
     mUpdateTimer = nullptr;
diff --git a/mailnews/news/src/nsNNTPProtocol.h b/mailnews/news/src/nsNNTPProtocol.h
index 08db18ee8c..6dd3c58dec 100644
--- a/mailnews/news/src/nsNNTPProtocol.h
+++ b/mailnews/news/src/nsNNTPProtocol.h
@@ -197,7 +197,7 @@ private:
 
   nsCOMPtr<nsIAsyncInputStream> mDisplayInputStream;
   nsCOMPtr<nsIAsyncOutputStream> mDisplayOutputStream;
-  nsMsgLineStreamBuffer   * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
+  RefPtr<nsMsgLineStreamBuffer> m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream
   // the nsINntpURL that is currently running
   nsCOMPtr<nsINntpUrl> m_runningURL;
   bool        m_connectionBusy;