341120 - Coverity 541 nss_cms_recipients_traverse leaks "rle"

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

In nss_cms_recipients_traverse, in case NSSCMSRecipientInfoID_KeyTrans,
we allocate a NSSCMSRecipient object and store its address in variable "rle".  

70 	rle = (NSSCMSRecipient *)PORT_ZAlloc(sizeof(NSSCMSRecipient));

Then in the default case of the nested switch, we return without freeing it.

85   			default:
86   			    PORT_SetError(SEC_ERROR_INVALID_ARGS);
87   			    return -1;
88   			}

Comment 1

[Alexei Volkov]

Attachment: check recipientIdentifier value before allocation

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 226555 [details] [diff] [review]
check recipientIdentifier value before allocation

There are two cases in this switch that seem nearly identical:

> 	case NSSCMSRecipientInfoID_KeyTrans:
> 	case NSSCMSRecipientInfoID_KeyAgree:

If an invalid identifierType is a potential problem for one, then 
it seems like it could also be a problem for the other.  
So, please apply your fix to both cases.  

Also, once you've tested that the identifier type can only be
one of those two values, then the switch on identifier type
can be replaced with a simpler if/then/else.

Comment 3

[Alexei Volkov]

there are three possible choices in first case(NSSCMSRecipientInfoID_KeyTrans), and we want to treat the third one as an error.

I didn't think of case NSSCMSRecipientInfoID_KeyAgree as a problem, since there is only limited usage of it's types mostly hard coded to NSSCMSKeyAgreeRecipientID_IssuerSN. asn1 decoder suppose to take care of correct value setting for keyAgreeRecipientInfo.recipientEncryptedKeys[].recipientIdentifier.identifierType for any recipientEncryptedKeys we will decode.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 226555 [details] [diff] [review]
check recipientIdentifier value before allocation

ok, then r+

Comment 5

[Alexei Volkov]

tip:
new revision: 1.5; previous revision: 1.4

3.11 branch:
new revision: 1.4.2.1; previous revision: 1.4