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

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;

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)

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;

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();

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");

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

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,
+          RefPtr<nsMsgLineStreamBuffer> inputStreamBuffer =
-                                                                               true,    // allocate new lines
+            new nsMsgLineStreamBuffer(FILE_IO_BUFFER_SIZE,
-                                                                               false);  // leave CRLFs on the returned string
+                                      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();
       }

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)

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.

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)

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();

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;

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;