Closed
Bug 504136
Opened 15 years ago
Closed 14 years ago
pkix_pl_CRL_Hashcode makes coverity sad
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.12.7
People
(Reporter: timeless, Assigned: timeless)
References
()
Details
(Keywords: coverity)
Attachments
(1 file, 1 obsolete file)
2.40 KB,
patch
|
timeless
:
review+
|
Details | Diff | Splinter Review |
559 pkix_pl_CRL_Hashcode( 560 PKIX_PL_Object *object, 561 PKIX_UInt32 *pHashcode, 562 void *plContext) 563 { 564 PKIX_PL_CRL *crl = NULL; 565 PKIX_UInt32 certHash; 566 SECItem *crlDer = NULL; 567 568 PKIX_ENTER(CRL, "pkix_pl_CRL_Hashcode"); 569 PKIX_NULLCHECK_TWO(object, pHashcode); 570 571 PKIX_CHECK(pkix_CheckType(object, PKIX_CRL_TYPE, plContext), 572 PKIX_OBJECTNOTCRL); 573 574 crl = (PKIX_PL_CRL *)object; 575 if (crl->adoptedDerCrl) { 576 crlDer = crl->adoptedDerCrl; 577 } else if (crl->nssSignedCrl && crl->nssSignedCrl->derCrl) { you're carefully checking nssSignedCrl and derCrl, which means you think it's possible for one or both of them to be null 578 crlDer = crl->nssSignedCrl->derCrl; 579 } which means there's room for an else clause. Let's assume we reach that else clause then crlDer is null... and we crash here: 581 if (crlDer->data) 582 PKIX_CHECK(pkix_hash(crlDer->data, crlDer->len, 583 &certHash, plContext), 584 PKIX_ERRORINHASH); let's assume that we didn't crash there, but that crlDer->data was null, then Coverity complains that certHash is returned unitialized here: 586 *pHashcode = certHash; 587 588 cleanup: 589 590 PKIX_RETURN(CRL); 591 }
Updated•15 years ago
|
Assignee: nobody → alexei.volkov.bugs
OS: Linux → All
Priority: -- → P2
Hardware: x86 → All
Target Milestone: --- → 3.12.5
Version: unspecified → 3.12.3
Assignee: alexei.volkov.bugs → timeless
Status: NEW → ASSIGNED
Attachment #430931 -
Flags: review?(nelson)
Comment 2•14 years ago
|
||
Comment on attachment 430931 [details] [diff] [review] proposal referring review requests for libPKIX to Alexei.
Attachment #430931 -
Flags: review?(nelson) → review?(alexei.volkov.bugs)
Comment 3•14 years ago
|
||
Comment on attachment 430931 [details] [diff] [review] proposal Patch is fine. Crl, as it used today can not be initialized without der. Der should be available through either crl->nssSignedCrl or clr->adoptedDerCrl. Returning CANNOTAQUIRECRLDER as an error code is also fine. Please also change the error description so it does not specify a particular function. r=alexei
Attachment #430931 -
Flags: review?(alexei.volkov.bugs) → review+
Attachment #430931 -
Attachment is obsolete: true
Attachment #431316 -
Flags: review+
Updated•14 years ago
|
Keywords: checkin-needed
Target Milestone: 3.12.5 → 3.12.7
Comment 5•14 years ago
|
||
Alexei, Timeless, The patch as submitted and reviewed doesn't even compile. I fixed it and committed it on trunk. Bug 504136 pkix_pl_CRL_Hashcode makes coverity sad Patch contributed by Timeless <timeless@mozdev.org>, r=alexei
You need to log in
before you can comment on or make changes to this bug.
Description
•