Last Comment Bug 353371 - Klocwork 91117 - Null Pointer Dereference in CERT_CertChainFromCert
: Klocwork 91117 - Null Pointer Dereference in CERT_CertChainFromCert
Status: RESOLVED FIXED
: klocwork
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11
: All All
: P2 normal (vote)
: 3.12
Assigned To: Alexei Volkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-19 13:01 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2006-10-09 11:46 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
check for null (2.13 KB, patch)
2006-09-29 12:15 PDT, Alexei Volkov
nelson: review+
Details | Diff | Review

Description Nelson Bolyard (seldom reads bugmail) 2006-09-19 13:01:49 PDT
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.
Comment 1 Alexei Volkov 2006-09-29 12:15:07 PDT
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.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-09-29 21:58:28 PDT
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;
>     }
Comment 3 Alexei Volkov 2006-10-02 11:26:11 PDT
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.
Comment 4 Alexei Volkov 2006-10-09 11:46:06 PDT
/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

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