662652 - Leak of SFTKDBHandle->passwordKey.data

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Matt Mueller]

Attachment: patch

When a password is set in sftkdb, either through sftkdb_ChangePassword or sftkdb_CheckPassword, it is not freed when the sftkdb handle is closed.
Patch attached.


Valgrind backtrace #1:
Leak_DefinitelyLost                                                                                                                                                       
20 bytes in 1 blocks are definitely lost in loss record 785 of 2,975                                                                                                      
  malloc (m_replacemalloc/vg_replace_malloc.c:904)                                                                                                                        
  PR_Malloc (/usr/local/google/home/x/src/nss-cvs/mozilla/nsprpub/Linux2.6_x86_64_glibc_PTH_64_DBG.OBJ/pr/src/malloc/../../../../pr/src/malloc/prmem.c:467)               
  PORT_Alloc_Util (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/util/secport.c:112)                                                                      
  sftkdb_passwordToKey (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/sftkpwd.c:95)                                                              
  sftkdb_ChangePassword (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/sftkpwd.c:1220)                                                           
  NSC_InitPIN (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:3431)                                                                      
  PK11_InitPin (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/pk11wrap/pk11auth.c:464)                                                                    
  crypto::(anonymous namespace)::NSSInitSingleton::OpenUserDB(FilePath const&, char const*) (/crypto/nss_util.cc:501)                                                     
  crypto::(anonymous namespace)::NSSInitSingleton::OpenTestNSSDB(FilePath const&, char const*) (/crypto/nss_util.cc:265)                                                  
  crypto::OpenTestNSSDB(FilePath const&, char const*) (/crypto/nss_util.cc:653)                                                                                           
  net::CertDatabaseNSSTest::SetUp() (/net/base/cert_database_nss_unittest.cc:110)                                                                                         
  void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/testing/gtest/src/gtest.cc:2090)                                                                                                                                                                    
  void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/testing/gtest/src/gtest.cc:2142)                                                                                                                                                                       


Valgrind backtrace #2:
Leak_DefinitelyLost                                                                                                                                                       
20 bytes in 1 blocks are definitely lost in loss record 784 of 2,975                                                                                                      
  malloc (m_replacemalloc/vg_replace_malloc.c:904)                                                                                                                        
  PR_Malloc (/usr/local/google/home/x/src/nss-cvs/mozilla/nsprpub/Linux2.6_x86_64_glibc_PTH_64_DBG.OBJ/pr/src/malloc/../../../../pr/src/malloc/prmem.c:467)               
  PORT_Alloc_Util (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/util/secport.c:112)                                                                      
  sftkdb_passwordToKey (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/sftkpwd.c:95)                                                              
  sftkdb_CheckPassword (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/sftkpwd.c:746)                                                             
  sftk_hasNullPassword (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:621)                                                              
  SFTK_SlotReInit (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:2344)                                                                  
  SFTK_SlotInit (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:2437)                                                                    
  nsc_CommonInitialize (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:2825)                                                             
  NSC_Initialize (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/softoken/pkcs11.c:2887)                                                                   
  secmod_ModuleInit (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/pk11wrap/pk11load.c:252)                                                               
  secmod_LoadPKCS11Module (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/pk11wrap/pk11load.c:492)                                                         
  SECMOD_LoadModule (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/pk11wrap/pk11pars.c:1121)                                                              
  SECMOD_LoadModule (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/pk11wrap/pk11pars.c:1156)                                                              
  nss_InitModules (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/nss/nssinit.c:461)                                                                       
  nss_Init (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/nss/nssinit.c:620)                                                                              
  NSS_InitReadWrite (/usr/local/google/home/x/src/nss-cvs/mozilla/security/nss/lib/nss/nssinit.c:722)                                                                     
  crypto::(anonymous namespace)::NSSInitSingleton::NSSInitSingleton() (/crypto/nss_util.cc:386)                                                                           
  base::DefaultLazyInstanceTraits<crypto::(anonymous namespace)::NSSInitSingleton>::New(void*) (/./base/lazy_instance.h:56)                                               
  base::LazyInstance<crypto::(anonymous namespace)::NSSInitSingleton, base::DefaultLazyInstanceTraits<crypto::(anonymous namespace)::NSSInitSingleton> >::Pointer() (/./base/lazy_instance.h:149)                                                                                                                                                   
  base::LazyInstance<crypto::(anonymous namespace)::NSSInitSingleton, base::DefaultLazyInstanceTraits<crypto::(anonymous namespace)::NSSInitSingleton> >::Get() (/./base/lazy_instance.h:132)                                                                                                                                                       
  crypto::EnsureNSSInit() (/crypto/nss_util.cc:596)                                                                                                                       
  net::CertDatabase::CertDatabase() (/net/base/cert_database_nss.cc:30)                                                                                                   
  net::CertDatabaseNSSTest::CertDatabaseNSSTest() (/net/base/cert_database_nss_unittest.cc:105)                                                                           
  net::CertDatabaseNSSTest_ImportServerCert_Test::CertDatabaseNSSTest_ImportServerCert_Test() (/net/base/cert_database_nss_unittest.cc:435)                               
  testing::internal::TestFactoryImpl<net::CertDatabaseNSSTest_ImportServerCert_Test>::CreateTest() (/testing/gtest/include/gtest/internal/gtest-internal.h:523)           
  testing::Test* testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) (/testing/gtest/src/gtest.cc:2090)                                                                       
  testing::Test* testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::TestFactoryBase, testing::Test*>(testing::internal::TestFactoryBase*, testing::Test* (testing::internal::TestFactoryBase::*)(), char const*) (/testing/gtest/src/gtest.cc:2142)                                                                          
  testing::TestInfo::Run() (/testing/gtest/src/gtest.cc:2331)                                                                                                             
  testing::TestCase::Run() (/testing/gtest/src/gtest.cc:2445)                                                                                                             
  testing::internal::UnitTestImpl::RunAllTests() (/testing/gtest/src/gtest.cc:4237)                                                                                       
  bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/testing/gtest/src/gtest.cc:2090)                                                                                                              
  bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/testing/gtest/src/gtest.cc:2142)                                                                                                                 
  testing::UnitTest::Run() (/testing/gtest/src/gtest.cc:3874)                                                                                                             
  base::TestSuite::Run() (/base/test/test_suite.cc:131)                                                                                                                   
  main (/net/base/run_all_unittests.cc:24)

Comment 1

[Wan-Teh Chang]

Attachment: patch v2 by Matt Mueller

The memory that handle->passwordKey.data points to is allocated
by the PORT_Alloc call in sftkdb_passwordToKey and assigned to
handle->passwordKey.data in sftkdb_switchKeys.  sftkpwd.c calls
PORT_ZFree to free that memory on the error ("loser") code paths.
So this patch is correct.

Patch checked in on the NSS trunk (NSS 3.13).

Checking in sftkdb.c;
/cvsroot/mozilla/security/nss/lib/softoken/sftkdb.c,v  <--  sftkdb.c
new revision: 1.29; previous revision: 1.28
done