Hi Bruno,
On Tue, 2021-08-10 at 17:28 -0300, Bruno Meneguele wrote:
> The variable "password" is not freed nor returned in case get_password()
> succeeds. Instead of using an intermediary variable ("pwd") for returning
> the value, use the same "password" var. Issue found by Coverity scan tool.
>
> src/evmctl.c:2565: leaked_storage: Variable "password" going out of scope
> leaks the storage it points to.
>
> Signed-off-by: Bruno Meneguele <bmeneg@xxxxxxxxxx>
> ---
> src/evmctl.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 7a6f2021aa92..b49c7910a4a7 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -2601,8 +2601,9 @@ static struct option opts[] = {
> static char *get_password(void)
> {
> struct termios flags, tmp_flags;
> - char *password, *pwd;
> + char *password;
> int passlen = 64;
> + bool err = false;
>
> password = malloc(passlen);
> if (!password) {
> @@ -2622,16 +2623,24 @@ static char *get_password(void)
> }
>
> printf("PEM password: ");
> - pwd = fgets(password, passlen, stdin);
> + if (fgets(password, passlen, stdin) == NULL) {
> + perror("fgets");
> + /* we still need to restore the terminal */
> + err = true;
> + }
>From the fgets manpage:
fgets() returns s on success, and NULL on error
or when end of file
occurs while no characters have been read.
> /* restore terminal */
> if (tcsetattr(fileno(stdin), TCSANOW, &flags) != 0) {
> perror("tcsetattr");
> + err = true;
> + }
> +
> + if (err) {
> free(password);
> return NULL;
> }
>
> - return pwd;
> + return password;
Wouldn't a simpler fix be to test "pwd" here?
if (!pwd)
free(password);
return pwd;
thanks,
Mimi
> }
>
> int main(int argc, char *argv[])
