Variable "cipherName" tracked as NULL was passed to a function that dereferences it. [@ PORT_Strdup - SSL_SecurityStatus]

RESOLVED FIXED in 3.11.1

Status

NSS
Libraries
P3
normal
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: timeless, Assigned: Alexei Volkov)

Tracking

({coverity, crash})

3.11
3.11.1
coverity, crash

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CID 202, crash signature, URL)

Attachments

(1 attachment)

826 bytes, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
found by coverity
(Reporter)

Comment 1

12 years ago
Created attachment 218797 [details] [diff] [review]
don't jump after looking
Attachment #218797 - Flags: review?(nelson)
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;
Attachment #218797 - Flags: review?(nelson) → review+
> If cipherName is ever NULL here, there is a BIG flag elsewhere in the 
Make that:  a big BUG
Severity: critical → normal
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
Target Milestone: --- → 3.11.1
(Assignee)

Comment 4

12 years ago
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
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Assignee: nobody → alexei.volkov.bugs
CID 202
Whiteboard: CID 202
Crash Signature: [@ PORT_Strdup - SSL_SecurityStatus]
You need to log in before you can comment on or make changes to this bug.