304361 - smime: possible memory corruption when encoding/decoding smime_encryptionkeypref_template

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Alexei Volkov]

File nss/lib/smime/smimeutil.c

The following template has two choice template entries that
has the same "selector" values. As a result, if choice #2
(that corresponds to NSSCMSRecipientKeyIdentifierTemplate) get encoded,
the encoder tries to work with data that related to CERT_IssuerAndSNTemplate
(id.issuerAndSN pointer) and ends up reading data from uninitialized pointer.

NSSSMIMEEncryptionKeyPref_IssuerSN in second choice should be replaced
with NSSSMIMEEncryptionKeyPref_RKeyID.


static const SEC_ASN1Template smime_encryptionkeypref_template[] = {
    { SEC_ASN1_CHOICE,
	  offsetof(NSSSMIMEEncryptionKeyPreference,selector), NULL,
	  sizeof(NSSSMIMEEncryptionKeyPreference) },
    { SEC_ASN1_POINTER | SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_XTRN | 0,
	  offsetof(NSSSMIMEEncryptionKeyPreference,id.issuerAndSN),
	  SEC_ASN1_SUB(CERT_IssuerAndSNTemplate),
	  NSSSMIMEEncryptionKeyPref_IssuerSN },
    { SEC_ASN1_POINTER | SEC_ASN1_CONTEXT_SPECIFIC | 1,
	  offsetof(NSSSMIMEEncryptionKeyPreference,id.recipientKeyID),
	  NSSCMSRecipientKeyIdentifierTemplate,
	  NSSSMIMEEncryptionKeyPref_IssuerSN },
    { SEC_ASN1_POINTER | SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_XTRN | 2,
	  offsetof(NSSSMIMEEncryptionKeyPreference,id.subjectKeyID),
	  SEC_ASN1_SUB(SEC_OctetStringTemplate),
	  NSSSMIMEEncryptionKeyPref_SubjectKeyID },
    { 0, }
};

Comment 1

[Alexei Volkov]

The template is used by smimeutil.c:NSS_SMIMEUtil_CreateSMIMEEncKeyPrefs and
smimeutil.c:NSS_SMIMEUtil_GetCertFromEncryptionKeyPreference to store or
retrieve certificate(key) using  issuerAndSN, recipient key ID, or subject key ID.
The only implemented option today issuerAndSN.
Others are yet to be implemented. This is the reason the problem was not
seen/reported before.

Template is local to the smimeutil.c. Should be no problem fixing it as suggested.

Comment 2

[Alexei Volkov]

Attachment: Patch replaces NSSSMIMEEncryptionKeyPref_IssuerSN with NSSSMIMEEncryptionKeyPref_RKeyID

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 193088 [details] [diff] [review]
Patch replaces NSSSMIMEEncryptionKeyPref_IssuerSN with NSSSMIMEEncryptionKeyPref_RKeyID

Looks right to me.  Please check it in on the trunk.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Alexei, please check this in today.

Comment 5

[Alexei Volkov]

Comment on attachment 193088 [details] [diff] [review]
Patch replaces NSSSMIMEEncryptionKeyPref_IssuerSN with NSSSMIMEEncryptionKeyPref_RKeyID

I've lost track of this bug. Asking for second review to integrate to 3.11 branch.

Comment 6

[Alexei Volkov]

tip:
/cvsroot/mozilla/security/nss/lib/smime/smimeutil.c,v  <--  smimeutil.c
new revision: 1.18; previous revision: 1.17

3.11 branch:
/cvsroot/mozilla/security/nss/lib/smime/smimeutil.c,v  <--  smimeutil.c
new revision: 1.16.28.4; previous revision: 1.16.28.3