From 11fd322dad1189d559fc921656f26beb11b64f75 Mon Sep 17 00:00:00 2001 From: m0n73 Date: Thu, 20 Aug 2020 13:13:38 -0400 Subject: [PATCH 1/2] Check whether the logfile is a regular file. --- fiche.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/fiche.c b/fiche.c index 99d140d..14b96d0 100644 --- a/fiche.c +++ b/fiche.c @@ -252,19 +252,29 @@ int fiche_run(Fiche_Settings settings) { } } - // Check if log file is writable (if set) + // Check if log file is valid and writable (if set) if ( settings.log_file_path ) { - // Create log file if it doesn't exist - FILE *f = fopen(settings.log_file_path, "a+"); - fclose(f); - - // Then check if it's accessible - if ( access(settings.log_file_path, W_OK) != 0 ) { - print_error("Log file not writable!"); - return -1; + struct stat log_file_st; + memset(&log_file_st, 0, sizeof(struct stat)); + + if ( stat(settings.log_file_path, &log_file_st) == 0 ) { + // Is the log file a regular file? + if ( !S_ISREG(log_file_st.st_mode) ) { + print_error("Log file is not valid!"); + return -1; + } + + // Can we write to it? + if ( access(settings.log_file_path, W_OK) != 0 ) { + print_error("Log file is not writable!"); + return -1; + } + } else { + // Log file doesn't exist - create it. + FILE *f = fopen(settings.log_file_path, "a+"); + fclose(f); } - } // Try to set domain name From 5ae011dfc30fb6ed7312c1c95658b9978623c2d6 Mon Sep 17 00:00:00 2001 From: m0n73 Date: Fri, 21 Aug 2020 14:19:10 -0400 Subject: [PATCH 2/2] Fixed incomplete privilege drop in perform_user_change(); hardened privelege dropping code and fixed error checking. --- fiche.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/fiche.c b/fiche.c index 14b96d0..6b1c5c7 100644 --- a/fiche.c +++ b/fiche.c @@ -34,6 +34,7 @@ Use netcat to push text - example: #include #include +#include #include #include #include @@ -414,22 +415,38 @@ static int perform_user_change(const Fiche_Settings *settings) { // Get user details const struct passwd *userdata = getpwnam(settings->user_name); - const int uid = userdata->pw_uid; - const int gid = userdata->pw_gid; - - if (uid == -1 || gid == -1) { + if (!userdata) { print_error("Could find requested user: %s!", settings->user_name); return -1; } + const uid_t uid = userdata->pw_uid; + const gid_t gid = userdata->pw_gid; + if (setgid(gid) != 0) { - print_error("Couldn't switch to requested user: %s!", settings->user_name); + print_error("Couldn't switch to requested group for user: %s!", settings->user_name); + } + + // Check if gid change actually happened. + if (getgid() != gid) { + print_error("Couldn't switch to requested group for user: %s!", settings->user_name); + } + + // We must re-initialize supplementary groups to avoid inheriting + // root's supplementary groups. + if (initgroups(settings->user_name, gid) != 0) { + print_error("Couldn't initialize supplementary groups for user: %s!", settings->user_name); } if (setuid(uid) != 0) { print_error("Couldn't switch to requested user: %s!", settings->user_name); } + // Check if uid change actually happened. + if (getuid() != uid) { + print_error("Couldn't switch to requested user: %s!", settings->user_name); + } + print_status("User changed to: %s.", settings->user_name); return 0;