1065990 - Dereferencing of a NULL pointer may occur on usage of Port_Znew or Port_ZAlloc for memory allocation.

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Sachin Kumar Gupta]

Attachment: Patch for the reported bug

File : nss/lib/crmf/respcli.c
Function : CMMF_CertRepContentGetResponseAtIndex
Line of Error: 95

Here "certResponse" is allocated memory by "PORT_ZNew", in case there is no sufficient memory so PORT_ZNew would return NULL, which is assigned back to "certResponse" , which is later being dereferenced without any null check in 
function "cmmf_CopyCertResponse" and thereafter.


NSS v3.16.3 code:

 certResponse = PORT_ZNew(CMMFCertResponse);
    rv = cmmf_CopyCertResponse(NULL, certResponse,
                               inCertRepContent->response[inIndex]);
    if (rv != SECSuccess) {
        CMMF_DestroyCertResponse(certResponse);
        certResponse = NULL;
    }
    return certResponse;


Recommended Code:

certResponse = PORT_ZNew(CMMFCertResponse);
    
   if(certResponse)
    { 
        rv = cmmf_CopyCertResponse(NULL, certResponse,
                               inCertRepContent->response[inIndex]);
        if (rv != SECSuccess) {
           CMMF_DestroyCertResponse(certResponse);
           certResponse = NULL;
        }
     }
    return certResponse;

There are a number of such instances , I am mentioning just one here for proper suggestions and guidance.

I am attaching a patch for the same.

Comment 1

[Sachin Kumar Gupta]

Reminder for bug review.

Comment 2

[Sachin Kumar Gupta]

Reminder for bug review.

Comment 3

[Sachin Kumar Gupta]

Reminder for bug review.

Comment 4

[Kai Engert (:KaiE:)]

Comment on attachment 8487844 [details] [diff] [review]
Patch for the reported bug

r=kaie

Comment 5

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nss/rev/d63d3b5212be

Comment 6

[Wan-Teh Chang]

Comment on attachment 8487844 [details] [diff] [review]
Patch for the reported bug

Review of attachment 8487844 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.

::: lib/crmf/respcli.c
@@ +98,5 @@
> +        if (rv != SECSuccess) {
> +            CMMF_DestroyCertResponse(certResponse);
> +	     certResponse = NULL;
> +        }
> +    }    

Nit: there are white spaces at the end of this line.
(There is also one on line 96, which comes from the
original code.)