Page MenuHome GnuPG

pinentry coverity static analysis reports
Closed, ResolvedPublic


coverity scan reported couple of issues for pinentry package. The should be addressed with the following patch-set:

Couple of them are hopefully obvious., but I was not sure with the following one:

Error: USE_AFTER_FREE (CWE-672): [#def9]
pinentry-1.1.1/tty/pinentry-tty.c:555: freed_arg: "fclose" frees "ttyfi".
pinentry-1.1.1/tty/pinentry-tty.c:562: use_closed_file: Calling "fileno" uses file handle "ttyfi" after closing it.
#  560|       }
#  561|   
#  562|->   if (terminal_save (fileno (ttyfi)) < 0)
#  563|       rc = -1;
#  564|   

Error: CPPCHECK_WARNING (CWE-415): [#def10]
pinentry-1.1.1/tty/pinentry-tty.c:588: error[doubleFree]: Resource handle 'ttyfi' freed twice.
#  586|     if (pinentry->ttyname)
#  587|       {
#  588|->       fclose (ttyfi);
#  589|         fclose (ttyfo);
#  590|       }

Event Timeline

gniibe triaged this task as Normal priority.
gniibe added a subscriber: gniibe.

Thank you. I'll take care of this.

Applied and pushed.

Jakuje reopened this task as Open.EditedApr 14 2021, 5:44 PM

Thank you for applying the provided changes!

I saw the last issue that I did not provide patch for was not addressed and in corner case, we can still happen to get double-fclose, which should be addressed with the following patch.

Edit: Updated the patch as the previous version was introducing fd leaks which I missed.

I hope last amendment is the following, which can happen if the tty can be opened only for reading but not for writing:

--- a/tty/pinentry-tty.c
+++ b/tty/pinentry-tty.c
@@ -583,7 +583,8 @@ tty_cmd_handler (pinentry_t pinentry)
   if (pinentry->ttyname)
       fclose (ttyfi);
-      fclose (ttyfo);
+      if (ttyfo)
+        fclose (ttyfo);

   return rc;

Actually, calling do_touch_file when some error(s) are not good.
Let me fix all the things.