From 2def3bb0e894c7833cee5c870b996ea3e31d5e6c Mon Sep 17 00:00:00 2001
From: Pierre Pronchery <khorben@defora.org>
Date: Mon, 27 Aug 2018 20:00:02 +0200
Subject: [PATCH] Avoid TOCTOU by using fstat() after open()

This also needs fileno() after fopen(). This time it addresses the code
for the client.

This is the third part of three for Coverity issue 1355235.
---
 client/config_init.c |  2 +-
 client/fwknop.c      | 17 +++++++++++------
 client/utils.c       |  6 +++---
 client/utils.h       |  2 +-
 server/utils.c       |  4 ++--
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/client/config_init.c b/client/config_init.c
index 90fd924a..3774cd11 100644
--- a/client/config_init.c
+++ b/client/config_init.c
@@ -696,7 +696,7 @@ set_rc_file(char *rcfile, fko_cli_options_t *options)
      * client consumes a proper rc file with strict permissions set (thanks
      * to Fernando Arnaboldi from IOActive for pointing this out).
     */
-    if(verify_file_perms_ownership(rcfile) != 1)
+    if(verify_file_perms_ownership(rcfile, -1) != 1)
         exit(EXIT_FAILURE);
 
     return;
diff --git a/client/fwknop.c b/client/fwknop.c
index 020c627f..34cacfb5 100644
--- a/client/fwknop.c
+++ b/client/fwknop.c
@@ -893,15 +893,18 @@ show_last_command(const char * const args_save_file)
     char args_str[MAX_LINE_LEN] = {0};
     FILE *args_file_ptr = NULL;
 
-    if(verify_file_perms_ownership(args_save_file) != 1)
-        return 0;
-
     if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
             args_save_file);
         return 0;
     }
 
+    if(verify_file_perms_ownership(args_save_file, fileno(args_file_ptr)) != 1)
+    {
+        fclose(args_file_ptr);
+        return 0;
+    }
+
     if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) {
         log_msg(LOG_VERBOSITY_NORMAL,
                 "Last fwknop client command line: %s", args_str);
@@ -928,15 +931,17 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
 
     memset(argv_new, 0x0, sizeof(argv_new));
 
-    if(verify_file_perms_ownership(args_save_file) != 1)
-        return 0;
-
     if ((args_file_ptr = fopen(args_save_file, "r")) == NULL)
     {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
                 args_save_file);
         return 0;
     }
+    if(verify_file_perms_ownership(args_save_file, fileno(args_file_ptr)) != 1)
+    {
+        fclose(args_file_ptr);
+        return 0;
+    }
     if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL)
     {
         args_str[MAX_LINE_LEN-1] = '\0';
diff --git a/client/utils.c b/client/utils.c
index 718f0b6d..62921bdf 100644
--- a/client/utils.c
+++ b/client/utils.c
@@ -56,17 +56,17 @@ static fko_protocol_t fko_protocol_array[] =
 };
 
 int
-verify_file_perms_ownership(const char *file)
+verify_file_perms_ownership(const char *file, int fd)
 {
     int res = 1;
 
-#if HAVE_STAT
+#if HAVE_FSTAT && HAVE_STAT
     struct stat st;
 
     /* Every file that the fwknop client deals with should be owned
      * by the user and permissions set to 600 (user read/write)
     */
-    if((stat(file, &st)) == 0)
+    if((fd >= 0 && fstat(fd, &st) == 0) || stat(file, &st) == 0)
     {
         /* Make sure it is a regular file
         */
diff --git a/client/utils.h b/client/utils.h
index e4c74fc5..d685b358 100644
--- a/client/utils.h
+++ b/client/utils.h
@@ -52,7 +52,7 @@
 
 /* Prototypes
 */
-int     verify_file_perms_ownership(const char *file);
+int     verify_file_perms_ownership(const char *file, int fd);
 int     resolve_dst_addr(const char *dns_str, struct addrinfo *hints,
             char *ip_str, size_t ip_bufsize, fko_cli_options_t *opts);
 short   proto_inttostr(int proto, char *proto_str, size_t proto_size);
diff --git a/server/utils.c b/server/utils.c
index 7e8bd00c..32b1bca4 100644
--- a/server/utils.c
+++ b/server/utils.c
@@ -111,13 +111,13 @@ is_valid_file(const char *path)
 int
 verify_file_perms_ownership(const char *file, int fd)
 {
-#if HAVE_FSTAT
+#if HAVE_FSTAT && HAVE_STAT
     struct stat st;
 
     /* Every file that fwknopd deals with should be owned
      * by the user and permissions set to 600 (user read/write)
     */
-    if(fstat(fd, &st) == 0)
+    if((fd >= 0 && fstat(fd, &st) == 0) || stat(file, &st) == 0)
     {
         /* Make sure it is a regular file
         */