587401 - pkix_pl_LdapCertStore_GetCert and pkix_pl_LdapCertStore_GetCRL leak requestArena when PKIX_CHECK triggers goto cleanup

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[timeless]

341 #define PKIX_CHECK(func, descNum) \
342     do { \
343         pkixErrorResult = (func); \
344         if (pkixErrorResult) { \
345             TRACE_CHECK_FAILURE((func), PKIX_ErrorText[descNum]) \
346             pkixErrorClass = pkixErrorResult->errClass; \
347             pkixErrorCode = descNum; \
348             goto cleanup; \
349         } \
350     } while (0)

833 pkix_pl_LdapCertStore_GetCRL(
842         PRArenaPool *requestArena = NULL;

882         PKIX_PL_NSSCALLRV
883             (CERTSTORE, requestArena, PORT_NewArena, (DER_DEFAULT_CHUNKSIZE));

903         if (issuerNames) {
909                 if (numNames > 0) {
910                         for (thisName = 0; thisName < numNames; thisName++) {
911                                 PKIX_CHECK(PKIX_List_GetItem
912                                 (issuerNames,
913                                 thisName,
914                                 (PKIX_PL_Object **)&issuer,
915                                 plContext),
916                                 PKIX_LISTGETITEMFAILED);

When a PKIX_CHECK fails (like the one above), we goto cleanup.

985         if (requestArena) {
986                 PKIX_PL_NSSCALL(CERTSTORE, PORT_FreeArena,
987                         (requestArena, PR_FALSE));
988         }

The above will destroy the requestArena but does not null it out, which means that it is not safe to merely call PORT_FreeArena in cleanup.

1020 cleanup:
1022         if (PKIX_ERROR_RECEIVED) {
1023                 PKIX_DECREF(filteredCRLs);
1024         }
1025 
1026         PKIX_DECREF(params);
1027         PKIX_DECREF(issuerNames);
1028         PKIX_DECREF(issuer);
1029         PKIX_DECREF(candidate);
1030         PKIX_DECREF(responses);
1031         PKIX_DECREF(unfilteredCRLs);
1032         PKIX_DECREF(lcs);
1033 
1034         PKIX_RETURN(CERTSTORE);
1035 }

Comment 1

[timeless]

also applies to pkix_pl_LdapCertStore_GetCert

Comment 2

[timeless]

Attachment: patch

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 466124 [details] [diff] [review]
patch

Review requests for patches to libpkix should go to alexei.volkov

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, and this bug has priority 'P2'.
:beurdouche, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.