Bug 500815 - [PEM] fix gcc warnings leading to undefined behavior
[PEM] fix gcc warnings leading to undefined behavior
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: nss (Show other bugs)
10
All Linux
low Severity medium
: ---
: ---
Assigned To: Elio Maldonado Batiz
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 501138
  Show dependency treegraph
 
Reported: 2009-05-14 07:05 EDT by Kamil Dudka
Modified: 2009-07-19 06:26 EDT (History)
2 users (show)

See Also:
Fixed In Version: 3.12.3.99.3-2.10.4.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-19 06:25:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Kamil Dudka 2009-05-14 07:05:34 EDT
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 14:32:00 EDT
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 15:15:40 EDT
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 15:50:33 EDT
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 01:18:40 EDT
(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 02:28:10 EDT
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 00:06:11 EDT
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-26 22:39:08 EDT
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-26 22:56:56 EDT
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 06:25:12 EDT
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 06:26:18 EDT
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.