1216993 - [CID 222164][CID 222165] Overwriting leaks storage

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Franziskus Kiefer [:franziskus]]

The passwords returned here [1] use allocated memory, so we have to free it before moving the pointer to the new one.

[1] https://dxr.mozilla.org/mozilla-central/source/security/nss/cmd/modutil/pk11.c?case=true&from=pk11.c#711

Comment 1

[Franziskus Kiefer [:franziskus]]

Attachment: Bug1216993.patch

in case the passwords don't match we stay in the for loop and have to free the two newpw variables.

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 8676864 [details] [diff] [review]
Bug1216993.patch

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

::: cmd/modutil/pk11.c
@@ +714,5 @@
>  		PR_fprintf(PR_STDOUT, msgStrings[PW_MATCH_MSG]);
> +		memset(newpw, 0, strlen(newpw));
> +		memset(newpw2, 0, strlen(newpw2));
> +		PORT_Free(newpw);
> +		PORT_Free(newpw2);

Don't you want PORT_ZFree here?

Comment 3

[Franziskus Kiefer [:franziskus]]

Attachment: Bug1216993.patch

of course... I was just too excited about the PORT_Frees further down. I changed them as well.

Comment 4

[Eric Rescorla (:ekr)]

Comment on attachment 8676903 [details] [diff] [review]
Bug1216993.patch

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

LGTM

Committed as: https://hg.mozilla.org/projects/nss/rev/c82113e56943