Last Comment Bug 486060 - sec_asn1d_parse_leaf uses argument uninitialized by caller pbe_PK11AlgidToParam
: sec_asn1d_parse_leaf uses argument uninitialized by caller pbe_PK11AlgidToParam
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.11
: All All
: P3 normal (vote)
: 3.12.3
Assigned To: Nelson Bolyard (seldom reads bugmail)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-03-30 20:19 PDT by Nelson Bolyard (seldom reads bugmail)
Modified: 2009-03-31 14:04 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 for NSS Trunk (untested) (1.01 KB, patch)
2009-03-30 20:19 PDT, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2009-03-30 20:19:27 PDT
Created attachment 370138 [details] [diff] [review]
Patch v1 for NSS Trunk (untested)

This bug was reported in bug 447882 comment 13 by 
Rich Megginson <richm@stanfordalumni.org>

Some code paths in pbe_PK11AlgidToParam call SEC_ASN1DecodeItem without
first initializing (zeroing) the structure that will receive the output.
This occasionally leads to a crash in sec_asn1d_parse_leaf due to a 
an uninitialized SECItem structure that has a non-null data pointer,
which is not the result of the parser.

Rich suggests the attached patch should fix it.  I'd like to have steps
to reproduce.
Comment 1 Robert Relyea 2009-03-31 13:38:52 PDT
Comment on attachment 370138 [details] [diff] [review]
Patch v1 for NSS Trunk (untested)

r+ looks safe
Comment 2 Rich Megginson 2009-03-31 13:47:06 PDT
Note that this doesn't cause a crash, at least none that I saw.  valgrind reports it as "Conditional jump or move depends on uninitialised value(s)" at line 1561 (which probably includes 1562 and 1563) in secasn1d.c:
http://mxr.mozilla.org/mozilla/source/security/nss/lib/util/secasn1d.c#1561
1560         /* Strip leading zeroes when target is unsigned integer */
1561         if (state->underlying_kind == SEC_ASN1_INTEGER && /* INTEGER   */
1562             item->len == 0 &&                             /* MSB       */
1563             item->type == siUnsignedInteger)              /* unsigned  */
Comment 3 Nelson Bolyard (seldom reads bugmail) 2009-03-31 14:04:40 PDT
Checking in pk11wrap/pk11pbe.c; new revision: 1.23; previous revision: 1.22

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