Thread (7 messages) 7 messages, 3 authors, 2021-08-16

Re: [PATCH ima-evm-utils] evmctl: fix memory leak in get_password

From: Bruno Meneguele <hidden>
Date: 2021-08-16 15:10:34

On Wed, Aug 11, 2021 at 02:28:37PM -0400, Ken Goldman wrote:
On 8/11/2021 1:31 PM, Mimi Zohar wrote:
quoted
On Wed, 2021-08-11 at 13:51 -0300, Bruno Meneguele wrote:
quoted
On Wed, Aug 11, 2021 at 10:52:00AM -0400, Mimi Zohar wrote:
quoted
quoted
-	return pwd;
+	return password;
Wouldn't a simpler fix be to test "pwd" here?
         if (!pwd)
                 free(password);
         return pwd;
The problem is on success, when 'pwd' is actually not NULL.
With that, I can't free(password). I would need to asprintf(pwd, ...) or
strndup(password). Because of that, I thought it would be cleaner to
remove 'password' completely.
I see.  So instead of "return pwd" as suggested above,

         if (!pwd) {
                 free(password);
                 password = NULL;  <== set or return NULL
         }

         return password;
That looks cleaner to me.

My style would be

	if (pwd == NULL)

which compiles to the same binary, but it less prone to error.

In addition, since this is reading from stdin

1 - Do you want the newline to be part of the password?
I would say 'yes'. AFAIK OpenSSL preserves the newline if it's present
in the input from <stdin>:

"The returned string is always NUL-terminated and the '\n' is preserved
if present in the input data" (BIO_gets() manpage from OpenSSL)

Also, if the user passed the password to the PEM file creation through
the arguments list (no newline) it can also do the same with evmctl.
2 = Why is an empty password an error?
Considering the item 1, I don't think we have an empty string in this
case.

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help