Last Comment Bug 436577 - uninitialized variable in sec_pkcs5CreateAlgorithmID
: uninitialized variable in sec_pkcs5CreateAlgorithmID
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.12
: All All
: P1 normal (vote)
: 3.12.1
Assigned To: Robert Relyea
:
:
Mentors:
Depends on:
Blocks: 401928 606005
  Show dependency treegraph
 
Reported: 2008-05-30 13:13 PDT by Justin Dolske [:Dolske]
Modified: 2010-10-20 15:43 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Don't hide the passed in parameter. (550 bytes, patch)
2008-06-04 15:00 PDT, Robert Relyea
wtc: review+
Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2008-05-30 13:13:37 PDT
I'm was poking through this code trying to figure out how to use NSS's PKSC#5 support, and ran across this. (Distilled from http://mxr.mozilla.org/seamonkey/source/security/nss/lib/pk11wrap/pk11pbe.c#583)

Line 613 creates a local |cipherAlgorithm| variable, masking the function argument. Its  value is then checked at line 623, but the value hasn't been set. (SECOidTag is an enum typedef, but afaik those don't get autoinitialized? My K&R book is not in reach :).

584 sec_pkcs5CreateAlgorithmID(SECOidTag algorithm, 
585                            SECOidTag cipherAlgorithm,
...
610     if (...) {
...
613         SECOidTag cipherAlgorithm;
...
622         if (...) {
623             if (cipherAlgorithm == SEC_OID_UNKNOWN) {
624                 goto loser;
625             }

Depending on preferred NSS style, the fix would be to either:

* Delete line 613, as the |cipherAlgorithm| arg isn't used outside the block
* Change line 613 to |SECOidTag tmpAlg = cipherAlgorithm;| and s/cipherAlgorithm/tmpAlg/ in the code below it.
Comment 1 Wan-Teh Chang 2008-05-30 15:29:51 PDT
I also reported this compiler warning in bug 401928 comment 60.
Comment 2 Robert Relyea 2008-06-04 15:00:19 PDT
Created attachment 323773 [details] [diff] [review]
Don't hide the passed in parameter.

The uninitialized variable is really the variable that is passed in from the application.

NOTE: I don't believe this will affect any current usage in Firefox. This only happens is PKCS5v2 is explicitly requested and the separate cipher is specified. Not all the NSS interfaces support that combination yet.

bob
Comment 3 Wan-Teh Chang 2008-06-04 15:02:18 PDT
Comment on attachment 323773 [details] [diff] [review]
Don't hide the passed in parameter.

r=wtc.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2008-06-09 19:54:09 PDT
Checking in pk11pbe.c; new revision: 1.21; previous revision: 1.20

Note You need to log in before you can comment on or make changes to this bug.