Closed
Bug 550208
Opened 15 years ago
Closed 15 years ago
ECDH_Derive calls mp_clear for uninitialized k when PORT_Alloc(...) fails
Categories
(NSS :: Libraries, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.13
People
(Reporter: timeless, Assigned: nelson)
References
(Blocks 1 open bug, )
Details
(Keywords: coverity, Whiteboard: FIPS)
Attachments
(1 file, 1 obsolete file)
961 bytes,
patch
|
timeless
:
review+
|
Details | Diff | Splinter Review |
558 ECDH_Derive(SECItem *publicValue,
559 ECParams *ecParams,
560 SECItem *privateValue,
561 PRBool withCofactor,
562 SECItem *derivedSecret)
568 mp_int k; /* to hold the private value */
584 if ((pointQ.data = PORT_Alloc(2*len + 1)) == NULL) goto cleanup;
622 cleanup:
623 mp_clear(&k);
Assignee | ||
Comment 2•15 years ago
|
||
The affected code is inside the FIPS boundary.
We have a branch on which we're making changes to this code, in preparation
for the time when we prepare this code for the next FIPS evaluation.
Any changes will go onto that branch, not onto the CVS trunk at this time.
Whiteboard: FIPS
Assignee | ||
Comment 3•15 years ago
|
||
Comment on attachment 430543 [details] [diff] [review]
proposal
The problem here is that the variable 'k' is initialized too late.
The line that initializes it comes immediately after the troublesome
goto.
> len = (ecParams->fieldID.size + 7) >> 3;
> pointQ.len = 2*len + 1;
>- if ((pointQ.data = PORT_Alloc(2*len + 1)) == NULL) goto cleanup;
>
> MP_DIGITS(&k) = 0;
> CHECK_MPI_OK( mp_init(&k) );
Just take that MP_DIGITS(&k) line and move it up much closer to the
beginning of the function. It's not necessary to also move the
call to mp_init.
Attachment #430543 -
Flags: review?(nelson) → review-
Assignee | ||
Comment 4•15 years ago
|
||
Timeless, I invite you to review this patch.
Assignee: timeless → nelson
Attachment #430543 -
Attachment is obsolete: true
Attachment #436894 -
Flags: review?(timeless)
Attachment #436894 -
Flags: review?(timeless) → review+
Assignee | ||
Comment 5•15 years ago
|
||
lib/freebl/ec.c; new revision: 1.20.8.3; previous revision: 1.20.8.2
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.13
You need to log in
before you can comment on or make changes to this bug.
Description
•