Closed Bug 341120 Opened 18 years ago Closed 18 years ago

Coverity 541 nss_cms_recipients_traverse leaks "rle"

Categories

(NSS :: Libraries, defect)

3.11.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.11.3

People

(Reporter: nelson, Assigned: alvolkov.bgs)

Details

(Keywords: coverity, memory-leak)

Attachments

(1 file)

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   			}
Attachment #226555 - Flags: review?(nelson)
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.
Attachment #226555 - Flags: review?(nelson) → review-
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 on attachment 226555 [details] [diff] [review]
check recipientIdentifier value before allocation

ok, then r+
Attachment #226555 - Flags: review- → review+
tip:
new revision: 1.5; previous revision: 1.4

3.11 branch:
new revision: 1.4.2.1; previous revision: 1.4
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: