From cea77b76b3fef912bd79e777f97d353aa50474b6 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Sun, 10 Nov 2019 21:59:52 -0500 Subject: 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 | 5 ++++- mailnews/compose/src/nsSmtpProtocol.cpp | 1 - mailnews/compose/src/nsSmtpProtocol.h | 2 +- mailnews/imap/src/nsImapMailFolder.cpp | 3 +-- mailnews/imap/src/nsImapProtocol.cpp | 1 - mailnews/imap/src/nsImapProtocol.h | 2 +- mailnews/imap/src/nsImapService.cpp | 8 ++++---- mailnews/local/src/nsMailboxProtocol.cpp | 4 ---- mailnews/local/src/nsMailboxProtocol.h | 2 +- mailnews/local/src/nsPop3Protocol.cpp | 4 ---- mailnews/local/src/nsPop3Protocol.h | 2 +- mailnews/news/src/nsNNTPProtocol.cpp | 4 ---- mailnews/news/src/nsNNTPProtocol.h | 2 +- 13 files changed, 14 insertions(+), 26 deletions(-) (limited to 'mailnews') diff --git a/mailnews/base/util/nsMsgLineBuffer.h b/mailnews/base/util/nsMsgLineBuffer.h index eff3f7c7e..6bacd45af 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 04347c1d0..522890c78 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 c8cbf1406..b1370d4be 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 m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream nsTArray m_addresses; uint32_t m_addressesLeft; diff --git a/mailnews/imap/src/nsImapMailFolder.cpp b/mailnews/imap/src/nsImapMailFolder.cpp index 4fade9d3f..da1411cd0 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 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 5e2639a5a..20cadc25c 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 5c4f43abd..32cf90e4c 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 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 5e097311e..1d97dec29 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 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 fad77aa93..0148133b3 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 e030c8d7c..36c892069 100644 --- a/mailnews/local/src/nsMailboxProtocol.h +++ b/mailnews/local/src/nsMailboxProtocol.h @@ -74,7 +74,7 @@ private: nsCOMPtr m_mailboxParser; // Local state for the current operation - nsMsgLineStreamBuffer * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream + RefPtr 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 825f45ab3..5d9d9145a 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 a332eae84..43a8aa4e8 100644 --- a/mailnews/local/src/nsPop3Protocol.h +++ b/mailnews/local/src/nsPop3Protocol.h @@ -318,7 +318,7 @@ private: nsCOMPtr m_nsIPop3Sink; nsCOMPtr m_pop3Server; - nsMsgLineStreamBuffer * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream + RefPtr 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 7c6314d52..f721e9747 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 08db18ee8..6dd3c58de 100644 --- a/mailnews/news/src/nsNNTPProtocol.h +++ b/mailnews/news/src/nsNNTPProtocol.h @@ -197,7 +197,7 @@ private: nsCOMPtr mDisplayInputStream; nsCOMPtr mDisplayOutputStream; - nsMsgLineStreamBuffer * m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream + RefPtr m_lineStreamBuffer; // used to efficiently extract lines from the incoming data stream // the nsINntpURL that is currently running nsCOMPtr m_runningURL; bool m_connectionBusy; -- cgit v1.2.3