212446 - Uninitialized memory read in nsNNTPNewsgroupList::ProcessXOVERLINE

Status: VERIFIED FIXED

(MailNews Core :: Networking: NNTP, defect)

Description

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

Doing:

-url news://news.mozilla.org/netscape.public.mozilla.mail-news when you don't
have that newsgroup setup prior yields:

    [W] UMR: Uninitialized memory read in
nsNNTPNewsgroupList::ProcessXOVERLINE(char const*,UINT *) {1 occurrence}
        Reading 4 bytes from 0x14c21694 (4 bytes at 0x14c21694 uninitialized)
        Address 0x14c21694 is 44 bytes into a 120 byte block at 0x14c21668
        Address 0x14c21694 points to a C++ new block in heap 0x02760000
        Thread ID: 0x450
        Error location
        nsNNTPNewsgroupList::ProcessXOVERLINE(char const*,UINT *)
[nsNNTPNewsgroupList.cpp:849]
        
            PRTime elapsedTime;
        
     =>     LL_SUB(elapsedTime, PR_Now(), m_lastStatusUpdate);
        
            if (LL_CMP(elapsedTime, >, MIN_STATUS_UPDATE_INTERVAL) ||
                lastIndex == totIndex)
        nsNNTPProtocol::ReadXover(nsIInputStream *,UINT) [nsNNTPProtocol.cpp:3609]
            mBytesReceivedSinceLastStatusUpdate += status;
          }
        
     =>   rv = m_newsgroupList->ProcessXOVERLINE(line, &status);
          NS_ASSERTION(NS_SUCCEEDED(rv), "failed to process the XOVERLINE");
        
          m_numArticlesLoaded++;
        nsNNTPProtocol::ProcessProtocolState(nsIURI *,nsIInputStream
*,UINT,UINT) [nsNNTPProtocol.cpp:5180]
                        break;
        
                    case NNTP_XOVER:
     =>                 status = ReadXover(inputStream, length);
                        break;
        
                    case NNTP_XOVER_RESPONSE:
        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]
            /* This event better not be on an event queue anymore. */
            PR_ASSERT(PR_CLIST_IS_EMPTY(&self->link));
        
     =>     result = self->handler(self);
            if (NULL != self->synchronousResult) {
                PR_Lock(self->lock);
                self->synchronousResult = result;
        PL_ProcessPendingEvents [plevent.c:606]
                    break;
        
                PR_LOG(event_lm, PR_LOG_DEBUG, ("$$$ processing event"));
     =>         PL_HandleEvent(event);
                PR_LOG(event_lm, PR_LOG_DEBUG, ("$$$ done processing event"));
            }
        
        md_TimerProc   [plevent.c:977]
        Allocation location
        new(UINT)      [new.cpp:23]
        nsNNTPNewsgroupListConstructor [nsMsgNewsFactory.cpp:69]
        NS_GENERIC_FACTORY_CONSTRUCTOR(nsNntpIncomingServer)
        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)
        
        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;
        nsComponentManagerImpl::CreateInstanceByContractID(char
const*,nsISupports *,nsID const&,void * *) [nsComponentManager.cpp:2015]
            if (NS_SUCCEEDED(rv))
            {
        
     =>         rv = factory->CreateInstance(aDelegate, aIID, aResult);
                NS_RELEASE(factory);
            }
            else
    nsCreateInstanceByContractID::()(nsID const&,void * *)const
[nsComponentManagerUtils.cpp:76]
                status = NS_GetComponentManager(getter_AddRefs(compMgr));
                if (compMgr)
                    status = compMgr->CreateInstanceByContractID(mContractID,
mOuter,
     =>                                                          aIID,
aInstancePtr);
                else if (NS_SUCCEEDED(status))
                    status = NS_ERROR_UNEXPECTED;
            }
    nsCOMPtr<nsINNTPNewsgroupList>::assign_from_helper(nsCOMPtr_helper
const&,nsID const&) [nsCOMPtr.h:965]
    nsCOMPtr<nsINNTPNewsgroupList>::=(nsCOMPtr_helper const&) [nsCOMPtr.h:587]
    nsNNTPProtocol::BeginReadXover(void) [nsNNTPProtocol.cpp:3411]
                   &m_firstPossibleArticle,
                   &m_lastPossibleArticle);
        
     =>     m_newsgroupList = do_CreateInstance(NS_NNTPNEWSGROUPLIST_CONTRACTID,
&rv);
            if (NS_FAILED(rv)) return -1;
        
            rv = m_newsgroupList->Initialize(m_runningURL, m_newsFolder);
    nsNNTPProtocol::ProcessProtocolState(nsIURI *,nsIInputStream *,UINT,UINT)
[nsNNTPProtocol.cpp:5168]
                        break;
        
                    case NNTP_XOVER_BEGIN:
     =>                 status = BeginReadXover();
                        break;
        
                    case NNTP_FIGURE_NEXT_CHUNK:
    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)

Comment 1

[timeless]

Attachment: init m_lastStatusUpdate

Comment 2

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

Also, reading a message yields:

    [W] UMR: Uninitialized memory read in
nsMsgNewsFolder::NotifyDownloadedLine(char const*,UINT) {1 occurrence}
        Reading 4 bytes from 0x0e555cc0 (4 bytes at 0x0e555cc0 uninitialized)
        Address 0x0e555cc0 is 184 bytes into a 328 byte block at 0x0e555c08
        Address 0x0e555cc0 points to a C++ new block in heap 0x02760000
        Thread ID: 0x540
        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]
        nsNNTPProtocol::ProcessProtocolState(nsIURI *,nsIInputStream
*,UINT,UINT) [nsNNTPProtocol.cpp:5164]
        nsMsgProtocol::OnDataAvailable(nsIRequest *,nsISupports *,nsIInputStream
*,UINT,UINT) [nsMsgProtocol.cpp:326]
        nsInputStreamPump::OnStateTransfer(void) [nsInputStreamPump.cpp:418]
        nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream *)
[nsInputStreamPump.cpp:321]
        nsInputStreamReadyEvent::EventHandler(PLEvent *) [nsStreamUtils.cpp:116]
        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]
        RDFServiceImpl::GetResource(nsACString const&,nsIRDFResource * *)
[nsRDFService.cpp:1097]
        nsMsgNewsFolder::AddNewsgroup(char const*,char const*,nsIMsgFolder * *)
[nsNewsFolder.cpp:228]
        nsMsgNewsFolder::CreateSubfolder(WORD const*,nsIMsgWindow *)
[nsNewsFolder.cpp:585]
        nsNntpIncomingServer::SubscribeToNewsgroup(char const*)
[nsNntpIncomingServer.cpp:778]
        nsNNTPProtocol::LoadUrl(nsIURI *,nsISupports *) [nsNNTPProtocol.cpp:1172]
        nsMsgProtocol::AsyncOpen(nsIStreamListener *,nsISupports *)
[nsMsgProtocol.cpp:550]
        nsNNTPProtocol::AsyncOpen(nsIStreamListener *,nsISupports *)
[nsNNTPProtocol.cpp:1005]

Comment 3

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

(Note, Timeless's patch fixes the previous UMR).

Comment 4

[(not reading, please use [e-mail] instead)]

Comment on attachment 127578 [details] [diff] [review]
init m_lastStatusUpdate

r/sr=sspitzer

thanks stephend (for the bug report) and timeless (for the fix)

Comment 5

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

Patch checked in to fix the UMR in nsNNTPNewsgroupList::ProcessXOVERLINE.

I'm going to split out Uninitialized memory read in
nsMsgNewsFolder::NotifyDownloadedLine into a new bug.

Comment 6

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

Fixed.

Comment 7

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

Verified FIXED using Purify (when I had it last).