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)

x86
All
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: timeless, Assigned: nelson)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity, Whiteboard: FIPS)

Attachments

(1 file, 1 obsolete file)

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);
Attached patch proposal (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #430543 - Flags: review?(nelson)
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
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-
Blocks: FIPS2010
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+
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.

Attachment

General

Creator:
Created:
Updated:
Size: