Bug 500815 - [PEM] fix gcc warnings leading to undefined behavior
Summary: [PEM] fix gcc warnings leading to undefined behavior
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: nss
Version: 10
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Elio Maldonado Batiz
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 501138
TreeView+ depends on / blocked
 
Reported: 2009-05-14 11:05 UTC by Kamil Dudka
Modified: 2009-07-19 10:26 UTC (History)
2 users (show)

Fixed In Version: 3.12.3.99.3-2.10.4.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-07-19 10:25:42 UTC


Attachments (Terms of Use)
proposed patch (5.51 KB, patch)
2009-05-14 11:05 UTC, Kamil Dudka
no flags Details | Diff

Description Kamil Dudka 2009-05-14 11:05:34 UTC
Created attachment 343942 [details]
proposed patch

Description of problem:
While building the PEM module I can see a lot of (potentially dangerous) gcc warnings.

Version-Release number of selected component (if applicable):
nss-3.12.3-7.fc12

Expected results:
no gcc warnings

Additional info:
patch ready

Comment 1 Elio Maldonado Batiz 2009-05-17 18:32:00 UTC
While reviewing this patch I noticed that in psession.c make_key creates an MD5Context and doesn't call MD5_DestroyContext to dispose of it. It doesn't clear the buffer first either. I'll log this as a separate memory leak bug.

Comment 2 Kamil Dudka 2009-05-17 19:15:40 UTC
Good catch. In fact I've not tested loading of a private key protected by password with valgrind at all yet.

Comment 3 Kamil Dudka 2009-05-17 19:50:33 UTC
Elio, please append the following one-line patch to your bugfix for psession.c:

diff -ruNp psession.c psession.c
--- psession.c  2009-05-17 21:43:26.148306000 +0200
+++ psession.c  2009-05-17 21:45:30.999391394 +0200
@@ -391,6 +391,7 @@ pem_mdSession_Login
     if (rv != SECSuccess)
         goto loser;

+    nss_ZFreeIf(output);
     return CKR_OK;

   loser:

Comment 4 Elio Maldonado Batiz 2009-05-18 05:18:40 UTC
(In reply to comment #3) Kamil, freeing output on success and returning CKR_OK may not be enough. Doesn't the arena need to be freed as well?

Comment 5 Kamil Dudka 2009-05-18 06:28:10 UTC
The pem_DestroyPrivateKey() function should do it for us already. I think this is actually wrong place to discuss psession's memory leaks, we should move to bug 501191. This bug is about gcc warnings. Sorry for starting it here :-)

Comment 6 Fedora Update System 2009-06-23 04:06:11 UTC
nss-3.12.3.99.3-2.11.3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/nss-3.12.3.99.3-2.11.3.fc11

Comment 7 Fedora Update System 2009-06-27 02:39:08 UTC
nss-3.12.3.99.3-2.11.3.fc11 has been pushed to the Fedora 11 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'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-6948

Comment 8 Fedora Update System 2009-06-27 02:56:56 UTC
nss-3.12.3.99.3-2.10.4.fc10 has been pushed to the Fedora 10 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'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7017

Comment 9 Fedora Update System 2009-07-19 10:25:12 UTC
nss-3.12.3.99.3-2.11.3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2009-07-19 10:26:18 UTC
nss-3.12.3.99.3-2.10.4.fc10 has been pushed to the Fedora 10 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.