43351 - Potential null ptr deref in nsNativeComponentLoader.cpp

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Mark G. Adams]

If the Unload() function of nsNativeComponentLoader is called when Init() hasn't
been (eg, if startup fails for some reason), this will crash due to mDllStore
being NULL.

Patch:
----------------------------------------
Index: nsNativeComponentLoader.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/components/nsNativeComponentLoader.cpp,v
retrieving revision 1.45
diff -u -r1.45 nsNativeComponentLoader.cpp
--- nsNativeComponentLoader.cpp 2000/06/03 09:45:55     1.45
+++ nsNativeComponentLoader.cpp 2000/06/21 20:25:04
@@ -982,7 +982,9 @@
     callData.when = aWhen;

     // Cycle through the dlls checking to see if they want to be unloaded
-    mDllStore->Enumerate(nsFreeLibraryEnum, &callData);
+    if( mDllStore ) {
+        mDllStore->Enumerate(nsFreeLibraryEnum, &callData);
+    }
     return NS_OK;
 }

Comment 1

[Ray Whitmer]

Please explain why it is proper to call Unload without calling Init, and why it 
would be bad for this to raise an exception.

Comment 2

[Mark G. Adams]

This is hit if nsComponentManagerImpl::init() fails when called from
NS_InitXPCOM(). Here's the call sequence:

NS_InitXPCOM()
- compMgr = new nsComponentManagerImpl();
- rv = compMgr->Init();
- if (NS_FAILED(rv)) NS_RELEASE(compMgr);
  - nsComponentManagerImpl())::~nsComponentManagerImpl()
    - nsComponentManagerImpl()::Shutdown()
      - nsComponentManagerImpl()::UnloadLibraries()
        - nsNativeComponentLoader::UnloadAll()
          - crash since nsNativeComponentLoader() never had Init() called, which
is normally called at the end of nsComponentManagerImpl::PlatformInit() (through
nsComponentManager::Init()) if everything succeeds

This was exposed by checking the return value of
nsComponentManagerImpl()::PlatformInit(). However, it would have happened later
anyways, just not as obviously.

Comment 3

[Ray Whitmer]

Thanks for the clarification that Unload was not being externally called, but 
only internally by the destructor.

I think this suggested change should be applied fairly soon.  It apparently does 
not prevent mozilla from executing correctly, but it creates a messy-looking 
crash in a case where it would have failed anyway.
is the crash that occurs when the component registry is not writeable.

Comment 4

[Ray Whitmer]

Applied the patch, exactly as given.

Comment 5

[Tracy Walker [:tracy]]

please verify

Comment 6

[leger]

Mark, please mark verified if all is well.  Fixed for you?