Closed
Bug 307293
Opened 19 years ago
Closed 19 years ago
Tag and type members of nsNSSASN1Sequence/nsNSSASN1PrintableItem remain uninitialized
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
People
(Reporter: sfraser_bugs, Assigned: KaiE)
Details
(Keywords: fixed1.8)
Attachments
(1 file)
1.11 KB,
patch
|
wtc
:
review+
rrelyea
:
superreview+
asa
:
approval1.8b5+
|
Details | Diff | Splinter Review |
nsNSSASN1Sequence and nsNSSASN1PrintableItem do not initialize their mType and mTag member variables, so later calls to GetType() and GetTag() return garbage. For some reason these are not being filled in when the objects are created.
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.8b5?
Reporter | ||
Comment 1•19 years ago
|
||
It seems that the nsNSSCertificate::CreateASN1Struct() code path does not initialize the tag and type fields.
Reporter | ||
Comment 2•19 years ago
|
||
Attachment #195094 -
Flags: superreview?(rrelyea)
Attachment #195094 -
Flags: review?(wtchang)
Comment 3•19 years ago
|
||
Comment on attachment 195094 [details] [diff] [review] Patch I can't say if 0 is the right initial value for mType and mTag. But I verified that this patch is correct C++ code and it does no harm. 0 is an invalid value for mTag (valid values are 0x01 - 0x1f). For mType, 0 either means ASN1_END_CONTENTS or is invalid. So this patch replaces random garbage by fixed garbage, which is an improvement.
Attachment #195094 -
Flags: review?(wtchang) → review+
Updated•19 years ago
|
Attachment #195094 -
Flags: superreview?(rrelyea) → superreview+
Comment 4•19 years ago
|
||
The mType and mTag members are set by the buildASN1ObjectFromDER static function in security/manager/ssl/src/nsNSSASN1Object.cpp, which is only called by CreateFromDER, but no code calls CreateFromDER. If no code (other than the constructors after the proposed patch is applied) sets the mTag and mType members, then we shouldn't call the GetType and GetTag methods either. So one can also view the mTag and mType members as dead code and they should be removed along with the CreateFromDER and buildASN1ObjectFromDER functions, and the GetType, SetType, GetTag, SetTag methods.
Comment 5•19 years ago
|
||
Simon, can you describe the user impact and the risk in taking this patch?
Reporter | ||
Comment 6•19 years ago
|
||
As far as I know, there is no user impact, as I think no-one is using the accessors for those uninitialized fields. This is simply a "correctness" fix.
Updated•19 years ago
|
Attachment #195094 -
Flags: approval1.8b5+
Reporter | ||
Comment 7•19 years ago
|
||
Fixed on trunk and branch.
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: blocking1.8b5?
Keywords: fixed1.8
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•