Closed Bug 301394 Opened 20 years ago Closed 19 years ago

Can't call NSC_Sign with a NULL pSignature

Categories

(NSS :: Libraries, defect)

3.10
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: wtc, Assigned: wtc)

Details

Attachments

(1 file)

I found this bug by code inspection. I haven't confirmed it with a test program. NSC_Sign, the C_Sign method of our softoken, will crash if the pSignature argument is NULL. I determined this by tracing the code for RSA, DSA, and ECDSA signing. They all eventually pass pSignature as the second argument to mp_to_fixlen_octets, which will dereference it. One can use a NULL pSignature to determine the length of the signature.
Comment on attachment 191411 [details] [diff] [review] Preliminary patch for ECDSA_SignDigestWithSeed This patch is correct, but I'd say it doesn't go far enough. If there's no place to put the signature, we shouldn't waste the CPU time to compute it, only to then discard it. We should short-cut as much of the code as possible, and just return the length as quickly as possible. I'm not sure it should even be necessary to go from softoken into freebl just to get the length, but I don't object to doing so.
Comment on attachment 191411 [details] [diff] [review] Preliminary patch for ECDSA_SignDigestWithSeed Nelson, Thank you for reviewing this patch voluntarily. I came to the same conclusion. I still attached this old patch because I was removing low-priority changes from my source tree. I agree the proper patch should shortcut as much code as possible.
Attachment #191411 - Attachment description: Patch for ECDSA_SignDigestWithSeed → Preliminary patch for ECDSA_SignDigestWithSeed
This bug is very much like bug 320187. Wan-Teh, does this bug apply only to ECDSA Signing? I ask because that is all your patch appears to address.
Whiteboard: ECC
This bug is invalid. NSC_Sign (and NSC_SignFinal) doesn't crash if pSignature is NULL. It handles a NULL pSignature like this: if (!pSignature) { *pulSignatureLen = context->maxLen; goto finish; } The only issue with the above code is that context->maxLen may be a generous estimate of the signature's length. This is the case for ECDSA signatures, for which context->maxLen is set to MAX_ECKEY_LEN * 2, regardless of the curve used. For RSA and DSA signatures, context->maxLen is the exact signature length (nsslowkey_PrivateModulusLen(privKey) and DSA_SIGNATURE_LEN, respectively). I have a better version of my "Preliminary patch for ECDSA_SignDigestWithSeed" in bug 320589.
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Whiteboard: ECC
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: