584875 - Contents of sec_PKCS12EncoderContext are only freed on error handling case.

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Matt Mueller]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US) AppleWebKit/534.3 (KHTML, like Gecko) Chrome/6.0.472.14 Safari/534.3
Build Identifier: both NSS 3.12.3.1 & hg mozilla-central trunk (as of 20100804)

sec_pkcs12_encoder_start_context creates a sec_PKCS12EncoderContext, and cleans it up if an error occurs.  If an error does not occur, it is never cleaned up.



Reproducible: Always

Steps to Reproduce:
Use SEC_PKCS12Encode under valgrind.
Actual Results:  
Leak_DefinitelyLost
1,171 (1,047 direct, 124 indirect) bytes in 1 blocks are definitely lost in loss record 763 of 782
  malloc (algrind-for-chromium-client/valgrind/scripts/valgrind-memcheck/coregrind/m_replacemalloc/vg_replace_malloc.c:236)
  malloc (base/process_util_linux.cc:607)
  PR_Malloc (uild/buildd/nspr-4.7.5/mozilla/nsprpub/pr/src/malloc/prmem.c:467)
  PL_ArenaAllocate (uild/buildd/nspr-4.7.5/mozilla/nsprpub/lib/ds/plarena.c:229)
  PORT_ArenaAlloc_Util (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/util/secport.c:246)
  PORT_ArenaZAlloc_Util (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/util/secport.c:268)
  sec_pkcs7_create_content_info (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/pkcs7/p7create.c:152)
  SEC_PKCS7CreateData (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/pkcs7/p7create.c:1255)
  sec_pkcs12_encoder_start_context (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/pkcs12/p12e.c:1556)
  SEC_PKCS12Encode (mp/nss.y17059/nss-3.12.3.1/mozilla/security/nss/lib/pkcs12/p12e.c:1954)
  mozilla_security_manager::nsPKCS12Blob_Export(std::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::vector<scoped_refptr<net::X509Certificate>, 
std::allocator<scoped_refptr<net::X509Certificate> > > const&, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const&) (net/third_party/mozilla_security_manager/nsPKCS12Blob.cpp:433)
  net::CertDatabase::ExportUserCertsToPKCS12(std::basic_string<char, std::char_traits<char>, std::allocator<char> >*, std::vector<scoped_refptr<net::X509Certificate>, std
::allocator<scoped_refptr<net::X509Certificate> > > const&, std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const&) (net/base/cert_database_nss.cc:107)
  net::CertDatabaseNSSTest_ImportFromPKCS12AndExportAgain_Test::TestBody() (net/base/cert_database_nss_unittest.cc:112)
  testing::Test::Run() (testing/gtest/src/gtest.cc:2095)
  testing::internal::TestInfoImpl::Run() (testing/gtest/src/gtest.cc:2314)
  testing::TestCase::Run() (testing/gtest/src/gtest.cc:2420)
  testing::internal::UnitTestImpl::RunAllTests() (testing/gtest/src/gtest.cc:4024)
  testing::UnitTest::Run() (testing/gtest/src/gtest.cc:3687)
  TestSuite::Run() (./base/test/test_suite.h:152)
  main (net/base/run_all_unittests.cc:27)


Expected Results:  
no leaks.

Attaching a patch.

Comment 1

[Matt Mueller]

Attachment: Clean up encoder context at the end of SEC_PKCS12Encode.

Comment 2

[Kai Engert [:KaiE:]]

=> NSS

Matt, thank you very much for filing these bugs and providing patches!

FYI, directory mozilla/security/nss is part of the NSS security library, which is tracked and released separately from the Mozilla application code.

Core / Security PSM is the directory mozilla/security/manager.

Comment 3

[Matt Mueller]

Just tested w/NSS 3.12.7, the problem still exists and the patch still applies.

Kai: sorry about that, I missed the "other" choice on the new bug page.

Comment 4

[Kai Engert [:KaiE:]]

(In reply to comment #3)
> 
> Kai: sorry about that, I missed the "other" choice on the new bug page.

No worries at all! Thanks for your contribution.

Comment 5

[Wan-Teh Chang]

Attachment: Clean up encoder context at the end of SEC_PKCS12Encode, v2, by Matt Mueller

I improved mattm's patch and checked it in on the
NSS trunk (NSS 3.13) and NSS_3_12_BRANCH (NSS 3.12.8).

Checking in p12e.c;
/cvsroot/mozilla/security/nss/lib/pkcs12/p12e.c,v  <--  p12e.c
new revision: 1.21; previous revision: 1.20
done

Checking in p12e.c;
/cvsroot/mozilla/security/nss/lib/pkcs12/p12e.c,v  <--  p12e.c
new revision: 1.20.32.1; previous revision: 1.20
done