1298712 - Fix up CertReq()

Status: RESOLVED DUPLICATE of bug 1317118

(NSS :: Libraries, defect)

Description

[Nicholas Nethercote [inactive]]

Coverity is reporting some problems in CertReq(): two resource leaks in error paths, and a missing failure check.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Fix minor resource leaks and a missing failure check in CertReq

Comment 2

[Martin Thomson [:mt:]]

Comment on attachment 8785729 [details] [diff] [review]
Fix minor resource leaks and a missing failure check in CertReq

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

I think that I might have seen a similar patch for the same problems elsewhere.

Comment 3

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8785729 [details] [diff] [review]
Fix minor resource leaks and a missing failure check in CertReq

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

Yeah, parts of this were fixed already. The patch should end up with only the first change after rebasing.

::: security/nss/cmd/certutil/certutil.c
@@ +217,5 @@
> +        if (rv != SECSuccess) {
> +            SECU_PrintError(progName, "Could not set signature algorithm id.");
> +            PORT_FreeArena(arena, PR_FALSE);
> +            return SECFailure;
> +        }

this got fixed already

@@ +225,5 @@
>      cr = CERT_CreateCertificateRequest(subject, spki, NULL);
>      SECKEY_DestroySubjectPublicKeyInfo(spki);
>      if (!cr) {
>          SECU_PrintError(progName, "unable to make certificate request");
> +        PORT_FreeArena(arena, PR_FALSE);

this one as well

Comment 4

[Nicholas Nethercote [inactive]]

Does this still need to be open?

Comment 5

[Franziskus Kiefer [:franziskus]]

This got fixed in bug 1317118