[CID 222164][CID 222165] Overwriting leaks storage

RESOLVED FIXED

Status

NSS
Libraries
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: fkiefer, Assigned: fkiefer)

Tracking

(Blocks: 1 bug, {coverity})

trunk
coverity

Firefox Tracking Flags

(firefox44 affected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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
Created attachment 8676864 [details] [diff] [review]
Bug1216993.patch

in case the passwords don't match we stay in the for loop and have to free the two newpw variables.
Attachment #8676864 - Flags: review?(martin.thomson)

Comment 2

2 years ago
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?
Attachment #8676864 - Flags: review?(martin.thomson)
Assignee: nobody → franziskuskiefer
Created attachment 8676903 [details] [diff] [review]
Bug1216993.patch

of course... I was just too excited about the PORT_Frees further down. I changed them as well.
Attachment #8676864 - Attachment is obsolete: true
Attachment #8676903 - Flags: review?(ekr)
Keywords: coverity

Comment 4

2 years ago
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
Attachment #8676903 - Flags: review?(ekr) → review+
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.