212596 - Uninitialized memory read in nsMsgNewsFolder::NotifyDownloadedLine

Status: RESOLVED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

[[e-mail] (gone - use [e-mail] instead)]

Just doing:

1.  mozilla.exe -url news://news.netscape.public.mozilla.test
2.  Clicking OK on the 'Download 500 headers'
3.  Selecting a message to read.

Yields:

    [W] UMR: Uninitialized memory read in
nsMsgNewsFolder::NotifyDownloadedLine(char const*,UINT) {1 occurrence}
        Reading 4 bytes from 0x0b37c8e0 (4 bytes at 0x0b37c8e0 uninitialized)
        Address 0x0b37c8e0 is 184 bytes into a 328 byte block at 0x0b37c828
        Address 0x0b37c8e0 points to a C++ new block in heap 0x02760000
        Thread ID: 0x47c
        Error location
        nsMsgNewsFolder::NotifyDownloadedLine(char const*,UINT)
[nsNewsFolder.cpp:1738]
            rv = StartNewOfflineMessage();
          }
        
     =>   m_numOfflineMsgLines++;
        
          if (m_tempMessageStream)
          {
        nsNNTPProtocol::DisplayArticle(nsIInputStream *,UINT)
[nsNNTPProtocol.cpp:2545]
                }
        
            if (m_newsFolder)
     =>       m_newsFolder->NotifyDownloadedLine(line, m_key);
        
                if (line[0] == '.' && line[1] == 0)
                {
        nsNNTPProtocol::ReadArticle(nsIInputStream *,UINT) [nsNNTPProtocol.cpp:2612]
            // otherwise we must be doing something like save to disk or cancel
            // in which case we are doing the work.
          if (m_channelListener)
     =>         return DisplayArticle(inputStream, length);
        
        
          char *line = m_lineStreamBuffer->ReadNextLine(inputStream, status,
pauseForMoreData);
        nsNNTPProtocol::ProcessProtocolState(nsIURI *,nsIInputStream
*,UINT,UINT) [nsNNTPProtocol.cpp:5164]
                        break;
        
                    case NNTP_READ_ARTICLE:
     =>                 status = ReadArticle(inputStream, length);
                        break;
        
                    case NNTP_XOVER_BEGIN:
        nsMsgProtocol::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsMsgProtocol.cpp:326]
        {
            // right now, this really just means turn around and churn through
the state machine
            nsCOMPtr<nsIURI> uri = do_QueryInterface(ctxt);
     =>     return ProcessProtocolState(uri, inStr, sourceOffset, count);
        }
        
        NS_IMETHODIMP nsMsgProtocol::OnStartRequest(nsIRequest *request,
nsISupports *ctxt)
        nsInputStreamPump::OnStateTransfer(void) [nsInputStreamPump.cpp:418]
                        seekable->Tell(&offsetBefore);
        
                    LOG(("  calling OnDataAvailable [offset=%u count=%u]\n",
mStreamOffset, avail));
     =>             rv = mListener->OnDataAvailable(this, mListenerContext,
mAsyncStream, mStreamOffset, avail);
        
                    // don't enter this code if ODA failed or called Cancel
                    if (NS_SUCCEEDED(rv) && NS_SUCCEEDED(mStatus)) {
        nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream *)
[nsInputStreamPump.cpp:321]
                    nextState = OnStateStart();
                    break;
                case STATE_TRANSFER:
     =>             nextState = OnStateTransfer();
                    break;
                case STATE_STOP:
                    nextState = OnStateStop();
        nsInputStreamReadyEvent::EventHandler(PLEvent *) [nsStreamUtils.cpp:116]
                nsInputStreamReadyEvent *ev = (nsInputStreamReadyEvent *) plevent;
                // bypass event delivery if this is a cleanup event...
                if (ev->mStream)
     =>             ev->mNotify->OnInputStreamReady(ev->mStream);
                ev->mNotify = 0;
                return NULL;
            }
        PL_HandleEvent [plevent.c:671]
        PL_ProcessPendingEvents [plevent.c:606]
        Allocation location
        new(UINT)      [new.cpp:23]
        nsMsgNewsFolderConstructor [nsMsgNewsFactory.cpp:70]
        NS_GENERIC_FACTORY_CONSTRUCTOR(nsNNTPArticleList)
        NS_GENERIC_FACTORY_CONSTRUCTOR(nsNNTPNewsgroupPost)
        NS_GENERIC_FACTORY_CONSTRUCTOR(nsNNTPNewsgroupList)
     => NS_GENERIC_FACTORY_CONSTRUCTOR(nsMsgNewsFolder)
        NS_GENERIC_FACTORY_CONSTRUCTOR(nsNewsDownloadDialogArgs)
        
        static const nsModuleComponentInfo components[] =
        nsGenericFactory::CreateInstance(nsISupports *,nsID const&,void * *)
[nsGenericFactory.cpp:86]
                                                       REFNSIID aIID, void
**aResult)
        {
            if (mInfo->mConstructor) {
     =>         return mInfo->mConstructor(aOuter, aIID, aResult);
            }
        
            return NS_ERROR_FACTORY_NOT_REGISTERED;
        RDFServiceImpl::GetResource(nsACString const&,nsIRDFResource * *)
[nsRDFService.cpp:1097]
            }
        
            nsIRDFResource *result;
     =>     rv = factory->CreateInstance(nsnull, NS_GET_IID(nsIRDFResource),
(void**) &result);
            if (NS_FAILED(rv)) return rv;
        
            // Now initialize it with it's URI. At this point, the resource
        nsMsgNewsFolder::AddNewsgroup(char const*,char const*,nsIMsgFolder * *)
[nsNewsFolder.cpp:228]
          uri.Append(escapedName.get());
        
          nsCOMPtr<nsIRDFResource> res;
     =>   rv = rdf->GetResource(uri, getter_AddRefs(res));
          if (NS_FAILED(rv)) return rv;
        
          nsCOMPtr<nsIMsgFolder> folder(do_QueryInterface(res, &rv));
        nsMsgNewsFolder::CreateSubfolder(WORD const*,nsIMsgWindow *)
[nsNewsFolder.cpp:585]
          path += hashedName.get();
        
          //Now let's create the actual new folder
     =>   rv = AddNewsgroup(newsgroupname.get(), "", getter_AddRefs(child));
        
          if (NS_SUCCEEDED(rv))
            SetNewsrcHasChanged(PR_TRUE); // subscribe UI does this - but maybe
we got here through auto-subscribe
        nsNntpIncomingServer::SubscribeToNewsgroup(char const*)
[nsNntpIncomingServer.cpp:778]
            rv = NS_MsgDecodeUnescapeURLPath(name, getter_Copies(newsgroupName));
            NS_ENSURE_SUCCESS(rv,rv);
        
     =>     rv = msgfolder->CreateSubfolder(newsgroupName.get(), nsnull);
            if (NS_FAILED(rv)) return rv;
        
            return NS_OK;
        nsNNTPProtocol::LoadUrl(nsIURI *,nsISupports *) [nsNNTPProtocol.cpp:1172]
        
                  if (confirmResult)
                  {
     =>             rv = m_nntpServer->SubscribeToNewsgroup(group.get());
                    containsGroup = PR_TRUE;
                  }
                  else {
    nsMsgProtocol::AsyncOpen(nsIStreamListener *,nsISupports *)
[nsMsgProtocol.cpp:550]
            // set the stream listener and then load the url
            m_channelContext = ctxt;
            m_channelListener = listener;
     =>     return LoadUrl(m_url, nsnull);
        }
        
        NS_IMETHODIMP nsMsgProtocol::GetLoadFlags(nsLoadFlags *aLoadFlags)
    nsNNTPProtocol::AsyncOpen(nsIStreamListener *,nsISupports *)
[nsNNTPProtocol.cpp:1005]
        
          }
          nsCOMPtr<nsIRequest> parentRequest;
     =>   return nsMsgProtocol::AsyncOpen(listener, ctxt);
        }
        
        nsresult nsNNTPProtocol::LoadUrl(nsIURI * aURL, nsISupports * aConsumer)

Comment 1

[timeless]

Attachment: only count offline when offline

Comment 2

[timeless]

Comment on attachment 127730 [details] [diff] [review]
only count offline when offline

StarNewOfflineMessage() is the function that initializes m_numOfflineMsgLines.
I believe stephend wasn't offline. I'm not 100% certain that
m_downloadMessageForOfflineUse is the right condition, but it's definitely the
one used in this function.

Comment 3

[David :Bienvenu]

Comment on attachment 127730 [details] [diff] [review]
only count offline when offline

sr=bienvenu, looks good

Comment 4

[David Bradley]

Is there a reason not to initialize m_numOfflineMsgLines in the constructor?

Comment 5

[timeless]

*** Bug 198712 has been marked as a duplicate of this bug. ***

Comment 6

[timeless]

.

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Timeless - ping... dbradley's got a q for you...

Comment 8

[David Bradley]

Comment on attachment 127730 [details] [diff] [review]
only count offline when offline

r=dbradley

I still think m_numOfflineMsgLines should be initialized in the constructor. But it wasn't before this patch, so not a big deal.

Comment 9

[timeless]

Comment on attachment 127730 [details] [diff] [review]
only count offline when offline

mozilla/mailnews/news/src/nsNewsFolder.cpp 	1.267