Closed
Bug 301394
Opened 20 years ago
Closed 19 years ago
Can't call NSC_Sign with a NULL pSignature
Categories
(NSS :: Libraries, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: wtc, Assigned: wtc)
Details
Attachments
(1 file)
933 bytes,
patch
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•20 years ago
|
||
Comment 2•20 years ago
|
||
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.
Assignee | ||
Comment 3•20 years ago
|
||
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
Comment 4•19 years ago
|
||
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
Assignee | ||
Comment 5•19 years ago
|
||
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.
Description
•