uninitialized variable in sec_pkcs5CreateAlgorithmID

RESOLVED FIXED in 3.12.1

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Dolske, Assigned: Robert Relyea)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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

9 years ago
I also reported this compiler warning in bug 401928 comment 60.
Assignee: nobody → rrelyea
Depends on: 401928
Version: trunk → 3.12

Updated

9 years ago
Blocks: 401928
No longer depends on: 401928
(Assignee)

Comment 2

9 years ago
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
Attachment #323773 - Flags: review?

Comment 3

9 years ago
Comment on attachment 323773 [details] [diff] [review]
Don't hide the passed in parameter.

r=wtc.
Attachment #323773 - Flags: review? → review+
Checking in pk11pbe.c; new revision: 1.21; previous revision: 1.20
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Target Milestone: --- → 3.12.1
Priority: -- → P1

Updated

7 years ago
Blocks: 606005
You need to log in before you can comment on or make changes to this bug.