Closed Bug 1256267 Opened 8 years ago Closed 8 years ago

Uninitialized scalar variable in fipstest.c

Categories

(NSS :: Test, defect)

defect
Not set
normal

Tracking

(firefox48 affected)

RESOLVED FIXED
Tracking Status
firefox48 --- affected

People

(Reporter: franziskus, Assigned: franziskus)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1355480)

Attachments

(1 file)

Function rsa_keypair_test in fipstest.c might use uninitialised variable |keySize|.

>  if ((rsakey = RSA_NewKey(keySize, &e)) == NULL) {
this is unlikely to happen, but this makes covertiy happy
Attachment #8733293 - Flags: review?(ttaubert)
Comment on attachment 8733293 [details] [diff] [review]
rsa_keypair_test.patch

Review of attachment 8733293 [details] [diff] [review]:
-----------------------------------------------------------------

::: cmd/fipstest/fipstest.c
@@ +5149,5 @@
>      FILE *rsareq;     /* input stream from the REQUEST file */
>      FILE *rsaresp;    /* output stream to the RESPONSE file */
>      int count;
>      int i;
> +    int keySize = 1;   /* key size in bits*/

Why initialize to 1? Maybe initialize to zero? Doesn't matter too much though probably.
Attachment #8733293 - Flags: review?(ttaubert) → review+
(In reply to Tim Taubert [:ttaubert] from comment #2)
> > +    int keySize = 1;   /* key size in bits*/
> 
> Why initialize to 1? Maybe initialize to zero? Doesn't matter too much
> though probably.

it actually does I think. RSA_NewKey checks for % 16 != 0. So 0 might lead to problems
Assignee: nobody → franziskuskiefer
https://hg.mozilla.org/projects/nss/rev/fe30ae3d4475
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #3)
> (In reply to Tim Taubert [:ttaubert] from comment #2)
> > > +    int keySize = 1;   /* key size in bits*/
> > 
> > Why initialize to 1? Maybe initialize to zero? Doesn't matter too much
> > though probably.
> 
> it actually does I think. RSA_NewKey checks for % 16 != 0. So 0 might lead
> to problems

Interesting, sounds like it's missing a check for |keySizeInBits > 0|? Passing 0 seems to make all dependent variables zero too and probably yields a few funny errors.
looking closer at this 0 is caught in |BAD_RSA_KEY_SIZE|, which checks expLen > keySize.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: