Red Hat Bugzilla – Bug 1174527
Segfault in pk12util when using -l option with certain .p12 files
Last modified: 2015-03-05 03:29:23 EST
Description of problem: Originally reprted by Hubert Kario while testing teh fix for Bug 1150645 - Importing an RSA private key fails if p < q Upstream reproducer causes Segmentation fault in nss when using -l option (in normal mode). Using: nss-3.16.2.3-1.el7.x86_64 nss-softokn-3.16.2.3-3.el7.x86_64 nspr-4.10.6-2.el7.x86_64 # valgrind pk12util -l Importable.p12 -w ImportablePwd.txt ==11611== Memcheck, a memory error detector ==11611== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==11611== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info ==11611== Command: pk12util -l Importable.p12 -w ImportablePwd.txt ==11611== ==11611== Invalid read of size 1 ==11611== at 0x508522E: sec_pkcs12_convert_item_to_unicode (p12local.c:931) ==11611== by 0x508AA00: sec_pkcs12_get_friendlyName (p12d.c:3038) ==11611== by 0x508AC3F: SEC_PKCS12DecoderIterateNext (p12d.c:3121) ==11611== by 0x4063FA: P12U_ListPKCS12File (pk12util.c:741) ==11611== by 0x404D9C: main (pk12util.c:1088) ==11611== Address 0x106fd5ddf is not stack'd, malloc'd or (recently) free'd ==11611== ==11611== ==11611== Process terminating with default action of signal 11 (SIGSEGV) ==11611== Access not within mapped region at address 0x106FD5DDF ==11611== at 0x508522E: sec_pkcs12_convert_item_to_unicode (p12local.c:931) ==11611== by 0x508AA00: sec_pkcs12_get_friendlyName (p12d.c:3038) ==11611== by 0x508AC3F: SEC_PKCS12DecoderIterateNext (p12d.c:3121) ==11611== by 0x4063FA: P12U_ListPKCS12File (pk12util.c:741) ==11611== by 0x404D9C: main (pk12util.c:1088) ==11611== If you believe this happened as a result of a stack ==11611== overflow in your program's main thread (unlikely but ==11611== possible), you can try to increase the size of the ==11611== main thread stack using the --main-stacksize= flag. ==11611== The main thread stack size used in this run was 8388608. ==11611== ==11611== HEAP SUMMARY: ==11611== in use at exit: 104,850 bytes in 731 blocks ==11611== total heap usage: 1,733 allocs, 1,002 frees, 616,806 bytes allocated ==11611== ==11611== LEAK SUMMARY: ==11611== definitely lost: 0 bytes in 0 blocks ==11611== indirectly lost: 0 bytes in 0 blocks ==11611== possibly lost: 46,353 bytes in 134 blocks ==11611== still reachable: 58,497 bytes in 597 blocks ==11611== suppressed: 0 bytes in 0 blocks ==11611== Rerun with --leak-check=full to see details of leaked memory ==11611== ==11611== For counts of detected and suppressed errors, rerun with: -v ==11611== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2) Segmentation fault Version-Release number of selected component (if applicable): How reproducible: on x86_64 system Steps to Reproduce: You need the test data from http://pkgs.devel.redhat.com/cgit/tests/nss-softokn/tree/Regression/bz1150645-Importing-an-RSA-private-key-fails-if-p-q 1. In a convinient directory execute rhpkg tests nss-softokn to get them This will bring the test files for nss-softokn 2. cd nss-softokn/Regression/bz1150645-Importing-an-RSA-private-key-fails-if-p-q 3. If you have the beaker test supprt exceute make run If not execute pk12util -l Importable.p12 -w ImportablePwd.txt Actual results: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff79872a6 in sec_pkcs12_convert_item_to_unicode (arena=0x0, dest=0x62b420, src=0x6322d8, zeroTerm=1, asciiConvert=0, toUnicode=0) at p12local.c:931 931 if((dest->data[dest->len-1] || dest->data[dest->len-2]) && zeroTerm) { Expected results: No segfault and subsewuently the .p12 filescan be imprted Additional info: Run pk12util -l Importable.p12 -w ImportablePwd.txt in the debugger When the sefault occurs get a backtrace (gdb) bt #0 0x00007ffff79872a6 in sec_pkcs12_convert_item_to_unicode (arena=0x0, dest=0x62b420, src=0x6322d8, zeroTerm=1, asciiConvert=0, toUnicode=0) at p12local.c:931 #1 0x00007ffff798c961 in sec_pkcs12_get_friendlyName (bag=bag@entry=0x62c8a8) at p12d.c:3038 #2 0x00007ffff798cba0 in SEC_PKCS12DecoderIterateNext (p12dcx=p12dcx@entry=0x62c570, ipp=ipp@entry=0x7fffffffdae8) at p12d.c:3121 #3 0x000000000040646b in P12U_ListPKCS12File (in_file=<optimized out>, slot=<optimized out>, slotPw=<optimized out>, p12FilePw=<optimized out>) at pk12util.c:741 #4 0x0000000000404eb2 in main (argc=1, argv=0x9c) at pk12util.c:1088 (gdb) nss-softokn is doesn't appear on the stacktrace. The problem is not with pk12util tool but rather withe the nss dedoding code for p12. I traced execution on the gdb for x86_64 (where it fails) and on i686 (where it doesn't) and on both the code patch followed is the same. gdb shows me that dest-len = 2, but dest-data is "" (on both). That causes the segfault.
Created attachment 969386 [details] simplified version of Hubert's test It can be used in system without Beaker and works wiithout using valgrind.
The results of my additiobal testing are as follows. I tested using Hubert's test with Beaker and my simplified version that doesn't require Beaker. 1) On rhel-6.6 and I can reproduce the segfault on x86_64, i686 is fine. 2) On fedora f21 I can reproduce the segfault on x86_64, i686 is fine. 3) Using my simpler test with the upstream source tree it segfaults on x86_64 and fine on i686 The stack trace is the one in Comment 0 I traced execution on the gdb for x86_64 (where it fails) and on i686 (where it doesn't) and on both the code path followed is the same. gdb shows me that dest-len = 2, but dest-data is "" (on both). That explains the segfault. How come it doesn't segfault on i686? With the following changes to nss/lib/pkcs12/p12local.c - if((dest->data[dest->len-1] || dest->data[dest->len-2]) && zeroTerm) { + if ((PORT_Strlen(dest->data) >= 2) && + (dest->data[dest->len-1] || dest->data[dest->len-2]) && + zeroTerm) { the segfault doesn't happen.
Created attachment 969387 [details] Gets rid of the segfault on x86_64
Created attachment 969405 [details] Gets rid of the segfault on x86_64 identical to the one submitted upstream
If the crash is because data->len-2 is negative, the better test is if ((data->len < 2) && ..... ) Rather than if ((strlen(data->data) <=2) && ... ) bob NOTE: if you want this in 7.1, you'd need to ask and justify it as a blocker.
(In reply to Bob Relyea from comment #6) > If the crash is because data->len-2 is negative, the better test is > > if ((data->len < 2) && ..... ) I think you meant to type > if ((dest->len < 2) && ..... )
and see my comment at: https://bugzilla.redhat.com/show_bug.cgi?id=1181614#c7
Created attachment 981212 [details] Fix the segfault - corrected version without breaking the python-nss sanity check as previous one did as explained in https://bugzilla.redhat.com/show_bug.cgi?id=1181614#c7
Created attachment 981584 [details] all changes including nss.spec changes
Comment on attachment 981212 [details] Fix the segfault - corrected version r+ rrelyea
*** Bug 1181614 has been marked as a duplicate of this bug. ***
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2015-0364.html