The default bug view has changed. See this FAQ.

Klocwork 91117 - Null Pointer Dereference in CERT_CertChainFromCert

RESOLVED FIXED in 3.12

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Alexei Volkov)

Tracking

({klocwork})

3.11
3.12
klocwork

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

2.13 KB, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
In CERT_CertChainFromCert in nss/lib/certhigh/certhigh.c, pointer 'stanCert' returned from call to function 'STAN_GetNSSCertificate' at line 1046 may be NULL and will be dereferenced by passing argument 1 to function 'NSSCertificate_BuildChain' at line 1050.
(Reporter)

Updated

11 years ago
Priority: -- → P2
Version: trunk → 3.11
Keywords: klocwork
(Assignee)

Comment 1

11 years ago
Created attachment 240652 [details] [diff] [review]
check for null

the actual problem seems to be in nssCertificate_BuildChain where 'c'(the cert parameter) is used before it checked for NULL. But if we got so far, nssCertificate_BuildChain will set wrong error code. So, still need to check if STAN_GetNSSCertificate returned non NULL value.
Assignee: nobody → alexei.volkov.bugs
Status: NEW → ASSIGNED
Attachment #240652 - Flags: review?(nelson)
(Reporter)

Comment 2

11 years ago
Comment on attachment 240652 [details] [diff] [review]
check for null

r=nelson.  One question:

>     stanCert = STAN_GetNSSCertificate(cert);
>+    if (!stanCert) {
>+        /* error code is set */
>+        return NULL;
>+    }
>     nssUsage.anyUsage = PR_FALSE;
>     nssUsage.nss3usage = usage;
>     nssUsage.nss3lookingForCA = PR_FALSE;
>     stanChain = NSSCertificate_BuildChain(stanCert, NULL, &nssUsage, NULL,
> 					  NULL, 0, NULL, NULL, td, cc);
>     if (!stanChain) {
> 	PORT_SetError(SEC_ERROR_UNKNOWN_ISSUER);

Is stanCert a counted reference?
Does this path leak stanCert?  
(OK, that's two questions.   :)

> 	return NULL;
>     }
Attachment #240652 - Flags: review?(nelson) → review+
(Assignee)

Comment 3

11 years ago
Nelson,

STAN_GetNSSCertificate returns a pointer to existent or newly created NSSCertificate structure. 
929     c = cc->nssCertificate;
930     if (c) {
931         return c;
932     }

Cert is not leaked in this case.
(Assignee)

Comment 4

11 years ago
/cvsroot/mozilla/security/nss/lib/certhigh/certhigh.c,v  <--  certhigh.c
new revision: 1.38; previous revision: 1.37
/cvsroot/mozilla/security/nss/lib/pki/certificate.c,v  <--  certificate.c
new revision: 1.60; previous revision: 1.59
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.