fill_CERTCertificateFields doesn't consistently order its checks of instance/context and nssCryptokiObject_Clone can leave ->label null

NEW
Assigned to

Status

8 years ago
8 years ago

People

(Reporter: timeless, Assigned: rrelyea)

Tracking

({coverity, memory-leak})

trunk
coverity, memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: FIPS)

(Reporter)

Description

8 years ago
98   	nssCryptokiObject_Clone (

103  	    rvObject = nss_ZNEW(NULL, nssCryptokiObject);
104  	    if (rvObject) {

if !object->label, !rvObject->label:
108  		if (object->label) {
109  		    rvObject->label = nssUTF8_Duplicate(object->label, NULL);

112  	    return rvObject;

632  	get_cert_instance(NSSCertificate *c)
641  		if (!instance) {
642  		    instance = nssCryptokiObject_Clone(*ci);
643  		} else {
651  			instance = nssCryptokiObject_Clone(*ci);

656  	    return instance;

721  	fill_CERTCertificateFields(NSSCertificate *c, CERTCertificate *cc, PRBool forced)

733  	    instance = get_cert_instance(c);

instance can be true:
735  	    if (instance) {
label can be false, see above:
736  		stanNick = instance->label;
so we don't check context:
737  	    } else if (context) {

stanNick is false:
741  	    if ((!cc->nickname && stanNick) || forced) {

context could be true:
772  	    if (context) {
so we don't do this:
785  	    } else if (instance) {
and we skipped this:
804  		nssCryptokiObject_Destroy(instance);

by which point we've leaking instance:
822  	}
Assignee: nobody → rrelyea
Whiteboard: FIPS
You need to log in before you can comment on or make changes to this bug.