Bug 596674 - [PEM] SIGSEGV within CreateObject()
Summary: [PEM] SIGSEGV within CreateObject()
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: nss
Version: 13
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Kamil Dudka
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 596783
TreeView+ depends on / blocked
 
Reported: 2010-05-27 09:49 UTC by Ales Kozumplik
Modified: 2014-09-30 23:39 UTC (History)
5 users (show)

Fixed In Version: nss-softokn-3.12.6-2.fc12.1
Clone Of:
: 596783 (view as bug list)
Environment:
Last Closed: 2010-06-21 13:06:08 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
my pkcs11.txt (451 bytes, text/plain)
2010-05-27 12:25 UTC, Ales Kozumplik
no flags Details
cert.pem causing the segv (2.93 KB, text/plain)
2010-05-27 12:37 UTC, Ales Kozumplik
no flags Details
proposed fix (4.46 KB, patch)
2010-05-27 13:46 UTC, Kamil Dudka
emaldona: review+
Details | Diff
proposed fixe with correction and in cvs formot (4.37 KB, patch)
2010-06-07 15:40 UTC, Elio Maldonado Batiz
no flags Details | Diff

Description Ales Kozumplik 2010-05-27 09:49:24 UTC
Description of problem:

Steps to Reproduce:
1. have a self-signed certificate with server's private key in cert.pem
2. run 'cert --cacert cert.pem URL'
  
Actual results:
sigsegv.

Expected results:
curl reports it can't connect to the server using the certificate.

Comment 1 Ales Kozumplik 2010-05-27 11:06:28 UTC
[akozumpl.redhat.com /etc/pki/nssdb]$ rpm -aq nss\* curl libcurl
nss-softokn-3.12.4-17.fc13.i686
nss-softokn-freebl-3.12.4-17.fc13.i686
libcurl-7.20.0-4.fc13.i686
nss_ldap-264-9.fc13.i686
curl-7.20.0-4.fc13.i686
nss-util-3.12.6-1.fc13.i686
nss-sysinit-3.12.6-4.fc13.i686
nss-mdns-0.10-8.fc12.i686
nss-3.12.6-4.fc13.i686

Comment 2 Ales Kozumplik 2010-05-27 12:25:14 UTC
Created attachment 417218 [details]
my pkcs11.txt

Comment 3 Ales Kozumplik 2010-05-27 12:37:25 UTC
Created attachment 417224 [details]
cert.pem causing the segv

Comment 4 Kamil Dudka 2010-05-27 13:29:07 UTC
#1  CreateObject (objClass=<value optimized out>, ...)                at pinst.c:239
#2  AddObjectIfNeeded (objClass=<value optimized out>, ...)           at pinst.c:307
#3  pem_CreateObject (fwInstance=<value optimized out>, ...)          at pobject.c:1147
#4  nssCKFWSession_CreateObject (fwSession=0x6a5e00, ...)             at session.c:1353
#5  NSSCKFWC_CreateObject (fwInstance=0x6a27a0, ...)                  at wrap.c:1991
#6  PK11_CreateNewObject (slot=0x6a49f0, session=1, ...)              at pk11obj.c:412
#7  PK11_CreateGenericObject (slot=0x6a49f0, ...)                     at pk11obj.c:1347
#8  nss_load_cert (ssl=0x62ac40, filename=0x62a590 "cert.pem", ...)   at nss.c:378
#9  Curl_nss_connect (conn=<value optimized out>, ...)                at nss.c:1218
#10 Curl_ssl_connect (conn=0x62ab00, sockindex=<value optimized out>) at sslgen.c:194
#11 Curl_http_connect (conn=0x62ab00, done=0x7fffffffd97e)            at http.c:1779
#12 Curl_protocol_connect (conn=0x62ab00, ...)                        at url.c:3281
#13 setup_conn (conn=0x62ab00, protocol_done=0x7fffffffd97e)          at url.c:4963
#14 Curl_async_resolved (conn=0x62ab00, ...)                          at url.c:5066
#15 connect_host (data=<value optimized out>)                         at transfer.c:1908
#16 Curl_perform (data=<value optimized out>)                         at transfer.c:2039
#17 operate (argc=<value optimized out>, argv=<value optimized out>)  at main.c:5214
#18 main (argc=<value optimized out>, argv=<value optimized out>)     at main.c:5539

Comment 5 Kamil Dudka 2010-05-27 13:46:26 UTC
Created attachment 417248 [details]
proposed fix

Comment 6 Elio Maldonado Batiz 2010-05-28 17:27:21 UTC
Comment on attachment 417248 [details]
proposed fix

The fix looks good. I have a few questions/comments.

Comment 7 Elio Maldonado Batiz 2010-05-28 17:39:40 UTC
(In reply to comment #0)
> Steps to Reproduce:
> 1. have a self-signed certificate with server's private key in cert.pem
> 2. run 'cert --cacert cert.pem URL'
I suppose you meant curl here :-). 
It probably doesn't matter which URL we use but could you give me a specific URL to try?

Comment 8 Elio Maldonado Batiz 2010-05-28 17:41:56 UTC
Kamil, Would you mind if integrate this patch into the PEM module submission to upstream we are currently reviewing?

Comment 9 Kamil Dudka 2010-05-28 19:02:45 UTC
(In reply to comment #7)
> I suppose you meant curl here :-).

Yes.

> It probably doesn't matter which URL we use but could you give me a specific
> URL to try?

An arbitrary (valid) https:// URL.  The problem here is really the cert, not URL.  The code expects a valid CA cert (or cert bundle).  It doesn't check anything and crashes badly on unexpected input.

The proposed patch should fix at least the reported SIGSEGV and a few OOM failures that are related to the place I touched.  But I admit the code of PEM reader has more such places.  It will be probably up to you to catch them and fix eventually...

(In reply to comment #8)
> Kamil, Would you mind if integrate this patch into the PEM module submission to
> upstream we are currently reviewing?    

That would be great.  As for the upstream review, it will take me more time to digest all the recent changes.

Comment 10 Kamil Dudka 2010-06-03 16:09:55 UTC
Oops, spotted a typo in the patch.  It's really ugly mistake, but it doesn't change anything as the code is actually dead:

diff --git a/pinst.c b/pinst.c
index f049465..70f5f4e 100644
--- a/pinst.c
+++ b/pinst.c
@@ -151,7 +151,7 @@ GetCertFields(unsigned char *cert, int cert_length,
     buf = issuer->data + issuer->len;

     /* only wanted issuer/SN */
-    if (subject == NULL && valid == NULL && subjkey == NULL) {
+    if (subject == NULL || valid == NULL || subjkey == NULL) {
         return SECSuccess;
     }
     /* validity */

Comment 11 Elio Maldonado Batiz 2010-06-07 15:40:47 UTC
Created attachment 421864 [details]
proposed fixe with correction and in cvs formot

The fix has been applied in nss-3.12.6-6.fc13, see 
http://koji.fedoraproject.org/koji/buildinfo?buildID=177031

Comment 12 Fedora Update System 2010-06-08 21:37:48 UTC
nss-softokn-3.12.6-2.fc12.1,nss-3.12.6-7.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/nss-softokn-3.12.6-2.fc12.1,nss-3.12.6-7.fc12

Comment 13 Fedora Update System 2010-06-10 19:20:37 UTC
nss-softokn-3.12.6-2.fc12.1, nss-3.12.6-7.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update nss-softokn nss'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/nss-softokn-3.12.6-2.fc12.1,nss-3.12.6-7.fc12

Comment 14 Fedora Update System 2010-06-21 13:05:49 UTC
nss-softokn-3.12.6-2.fc12.1, nss-3.12.6-7.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.