Last Comment Bug 334459 - Variable "cipherName" tracked as NULL was passed to a function that dereferences it. [@ PORT_Strdup - SSL_SecurityStatus]
: Variable "cipherName" tracked as NULL was passed to a function that dereferen...
Status: RESOLVED FIXED
CID 202
: coverity, crash
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11
: All All
: P3 normal (vote)
: 3.11.1
Assigned To: Alexei Volkov
:
Mentors:
http://bonsai.mozilla.org/cvsblame.cg...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-18 01:49 PDT by timeless
Modified: 2011-06-13 10:01 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
don't jump after looking (826 bytes, patch)
2006-04-18 01:52 PDT, timeless
nelson: review+
Details | Diff | Splinter Review

Description timeless 2006-04-18 01:49:18 PDT
found by coverity
Comment 1 timeless 2006-04-18 01:52:49 PDT
Created attachment 218797 [details] [diff] [review]
don't jump after looking
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-04-18 05:55:35 PDT
Comment on attachment 218797 [details] [diff] [review]
don't jump after looking

If cipherName is ever NULL here, there is a BIG flag elsewhere in the 
SSL code.  It would require that ss->sec.cipherType contain an invalid
value, outside of the range of the arrays ssl_cipherName and 
ssl3_cipherNamem which would indicate a coding error in libSSL.

This patch does avoid that crash, but it should be more aggressive.
We should put in an assertion that cipherName is not NULL.
So add one more line, right here.


>-	if (cipherName && PORT_Strstr(cipherName, "DES")) isDes = PR_TRUE;

        PORT_Assert(cipherName);

>+	if (cipherName) {
>+            if (PORT_Strstr(cipherName, "DES")) isDes = PR_TRUE;
> 
>-	if (cp) {
>-	    *cp = PORT_Strdup(cipherName);
>-	}
>+            if (cp) {
>+                *cp = PORT_Strdup(cipherName);
>+            }
>+        }
> 
> 	if (kp0) {
> 	    *kp0 = ss->sec.keyBits;
Comment 3 Nelson Bolyard (seldom reads bugmail) 2006-04-18 06:22:35 PDT
> If cipherName is ever NULL here, there is a BIG flag elsewhere in the 
Make that:  a big BUG
Comment 4 Alexei Volkov 2006-04-19 17:23:46 PDT
tip:
/cvsroot/mozilla/security/nss/lib/ssl/sslauth.c,v  <--  sslauth.c
new revision: 1.16; previous revision: 1.15

3.11 branch:
/cvsroot/mozilla/security/nss/lib/ssl/sslauth.c,v  <--  sslauth.c
new revision: 1.15.2.1; previous revision: 1.15
Comment 5 Nelson Bolyard (seldom reads bugmail) 2006-06-10 19:31:16 PDT
CID 202

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