Closed Bug 1065990 Opened 6 years ago Closed 6 years ago

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

Categories

(NSS :: Libraries, defect)

3.16.3
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
3.17.3

People

(Reporter: sachingpt999, Unassigned)

Details

Attachments

(1 file)

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.
Attachment #8487844 - Flags: review?(kaie)
Reminder for bug review.
Reminder for bug review.
Flags: needinfo?(ryan.sleevi)
Flags: needinfo?(rrelyea)
Reminder for bug review.
Flags: needinfo?(kaie)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(kaie)
Flags: needinfo?(ryan.sleevi)
Flags: needinfo?(rrelyea)
Comment on attachment 8487844 [details] [diff] [review]
Patch for the reported bug

r=kaie
Attachment #8487844 - Flags: review?(kaie) → review+
https://hg.mozilla.org/projects/nss/rev/d63d3b5212be
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.17.3
Target Milestone: 3.17.3 → 3.18
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.)
Attachment #8487844 - Flags: review+
Target Milestone: 3.18 → 3.17.3
You need to log in before you can comment on or make changes to this bug.