From a2d14f844d16dc242a46fcae50b77dd83daa86cd Mon Sep 17 00:00:00 2001 From: McDope Date: Mon, 8 Jul 2024 19:18:40 +0200 Subject: [PATCH] Enhancement: Fix issues identified by deepseek-coder-v2 (#229) - fixes issues identified by deepseek-coder-v2 - tiny enhancements --- src/conf.c | 38 +++++++++++++++---------- src/device.c | 17 ++++++----- src/local.c | 23 +++++++++++---- src/pad.c | 79 ++++++++++++++++++++++----------------------------- src/pam.c | 77 +++++++++++++++++++++++++------------------------ src/process.c | 75 +++++++++++++++++++++++------------------------- src/process.h | 2 +- src/tmux.c | 46 ++++++++++++++++++------------ src/volume.c | 18 ++++++++---- 9 files changed, 202 insertions(+), 173 deletions(-) diff --git a/src/conf.c b/src/conf.c index 1bfa45c4..57481632 100644 --- a/src/conf.c +++ b/src/conf.c @@ -52,14 +52,14 @@ static int pusb_conf_parse_options( char *xpath = NULL; size_t xpath_size; int i; - + // these can come from argv, so make sure nothing messes up snprintf later char xpath_user[32] = { }; char xpath_service[32] = { }; - snprintf(xpath_user, 32, "%s", user); - snprintf(xpath_service, 32, "%s", service); + snprintf(xpath_user, sizeof(xpath_user), "%s", user); + snprintf(xpath_service, sizeof(xpath_service), "%s", service); - struct s_opt_list opt_list[] = { + struct s_opt_list opt_list[] = { { CONF_DEVICE_XPATH, opts->device.name }, { CONF_USER_XPATH, xpath_user }, { CONF_SERVICE_XPATH, xpath_service }, @@ -71,12 +71,16 @@ static int pusb_conf_parse_options( { xpath_size = strlen(opt_list[i].name) + strlen(opt_list[i].value) + 1; xpath = xmalloc(xpath_size); + if (xpath == NULL) { + log_error("Memory allocation failed\n"); + return 0; + } memset(xpath, 0x00, xpath_size); snprintf(xpath, xpath_size, opt_list[i].name, opt_list[i].value, ""); pusb_conf_options_get_from(opts, xpath, doc); xfree(xpath); } - return (1); + return 1; } static int pusb_conf_device_get_property( @@ -93,11 +97,15 @@ static int pusb_conf_device_get_property( xpath_len = strlen(CONF_DEVICE_XPATH) + strlen(opts->device.name) + strlen(property) + 1; xpath = xmalloc(xpath_len); + if (xpath == NULL) { + log_error("Memory allocation failed\n"); + return 0; + } memset(xpath, 0x00, xpath_len); snprintf(xpath, xpath_len, CONF_DEVICE_XPATH, opts->device.name, property); retval = pusb_xpath_get_string(doc, xpath, store, size); xfree(xpath); - return (retval); + return retval; } static int pusb_conf_parse_device( @@ -108,13 +116,13 @@ static int pusb_conf_parse_device( pusb_conf_device_get_property(opts, doc, "vendor", opts->device.vendor, sizeof(opts->device.vendor)); pusb_conf_device_get_property(opts, doc, "model", opts->device.model, sizeof(opts->device.model)); - if (!pusb_conf_device_get_property(opts, doc, "serial", opts->device.serial, sizeof(opts->device.serial))) + if (!pusb_conf_device_get_property(opts, doc, "serial", opts->device.serial, sizeof(opts->device.serial))) { - return (0); + return 0; } pusb_conf_device_get_property(opts, doc, "volume_uuid", opts->device.volume_uuid, sizeof(opts->device.volume_uuid)); - return (1); + return 1; } int pusb_conf_init(t_pusb_options *opts) @@ -125,10 +133,10 @@ int pusb_conf_init(t_pusb_options *opts) if (uname(&u) == -1) { log_error("uname: %s\n", strerror(errno)); - return (0); + return 0; } snprintf(opts->hostname, sizeof(opts->hostname), "%s", u.nodename); - if (strnlen(u.nodename, sizeof(u.nodename)) > sizeof(opts->hostname)) + if (strnlen(u.nodename, sizeof(u.nodename)) > sizeof(opts->hostname)) { log_info("Hostname \"%s\" is too long, truncating to \"%s\".\n", u.nodename, opts->hostname); } @@ -143,7 +151,7 @@ int pusb_conf_init(t_pusb_options *opts) opts->one_time_pad = 1; opts->pad_expiration = 3600; opts->deny_remote = 1; - return (1); + return 1; } int pusb_conf_parse( @@ -161,12 +169,12 @@ int pusb_conf_parse( if (strnlen(user, sizeof(user)) > CONF_USER_MAXLEN) { log_error("Username \"%s\" is too long (max: %d).\n", user, CONF_USER_MAXLEN); - return (0); + return 0; } if (!(doc = xmlReadFile(file, NULL, 0))) { log_error("Unable to parse \"%s\".\n", file); - return (0); + return 0; } snprintf(device_xpath, sizeof(device_xpath), CONF_USER_XPATH, user, "device"); retval = pusb_xpath_get_string( @@ -180,7 +188,7 @@ int pusb_conf_parse( log_error("No authentication device configured for user \"%s\".\n", user); xmlFreeDoc(doc); xmlCleanupParser(); - return (0); + return 0; } if (!pusb_conf_parse_options(opts, doc, user, service)) { diff --git a/src/device.c b/src/device.c index 8555de69..def8548c 100644 --- a/src/device.c +++ b/src/device.c @@ -44,17 +44,17 @@ static int pusb_device_connected(t_pusb_options *opts, UDisksClient *udisks) { drive = udisks_object_get_drive(object); retval = strcmp(udisks_drive_get_serial(drive), opts->device.serial) == 0; - - if (strcmp(opts->device.vendor, "Generic") != 0) + + if (strcmp(opts->device.vendor, "Generic") != 0) { retval = retval && strcmp(udisks_drive_get_vendor(drive), opts->device.vendor) == 0; } - if (strcmp(opts->device.model, "Generic") != 0) + if (strcmp(opts->device.model, "Generic") != 0) { retval = retval && strcmp(udisks_drive_get_model(drive), opts->device.model) == 0; } - + g_object_unref(drive); if (retval) { break; @@ -62,16 +62,19 @@ static int pusb_device_connected(t_pusb_options *opts, UDisksClient *udisks) } } - if (retval) + if (retval) { log_info("Authentication device \"%s\" is connected.\n", opts->device.name); } - else + else { log_error("Authentication device \"%s\" is not connected.\n", opts->device.name); } - g_list_foreach(objects, (GFunc) g_object_unref, NULL); + for (i = 0; i < g_list_length(objects); ++i) + { + g_object_unref(g_list_nth(objects, i)->data); + } g_list_free(objects); return (retval); diff --git a/src/local.c b/src/local.c index cddc3730..b0e115f5 100644 --- a/src/local.c +++ b/src/local.c @@ -123,6 +123,7 @@ char *pusb_get_tty_from_display_server(const char *display) xfree(fd_path); xfree(link_path); xfree(fd_target); + closedir(d_proc); return NULL; } @@ -221,6 +222,11 @@ char *pusb_get_tty_by_loginctl() else { log_debug(" 'loginctl' returned nothing.\n"); + if (pclose(fp)) + { + log_debug(" Closing pipe for 'loginctl' failed, this is quite a wtf...\n"); + } + return (0); } } @@ -234,7 +240,7 @@ int pusb_is_loginctl_local() if ((fp = popen(loginctl_cmd, "r")) == NULL) { log_debug(" Opening pipe for 'loginctl' failed, this is quite a wtf...\n"); - return (0); + return 0; } char *is_remote = NULL; @@ -250,17 +256,17 @@ int pusb_is_loginctl_local() if (strcmp(is_remote, "no") == 0) { - return (1); + return 1; } else { - return (0); + return -1; } } else { log_debug(" 'loginctl' returned nothing.\n"); - return (0); + return 0; } } @@ -288,7 +294,7 @@ int pusb_local_login(t_pusb_options *opts, const char *user, const char *service while (pid != 0) { - pusb_get_process_name(pid, name); + pusb_get_process_name(pid, name, BUFSIZ); log_debug(" Checking pid %6d (%s)...\n", pid, name); if (strstr(name, "tmux") != NULL) @@ -386,11 +392,16 @@ int pusb_local_login(t_pusb_options *opts, const char *user, const char *service log_debug(" Trying to check for remote access by loginctl\n"); int loginctl_remote = pusb_is_loginctl_local(); - if (loginctl_remote != 0) + if (loginctl_remote == 1) { log_debug(" loginctl says this session is local\n"); local_request = 1; } + else if (loginctl_remote == -1) + { + log_debug(" loginctl says this session is remote\n"); + return 0; + } else { log_debug(" Trying to get tty by loginctl\n"); diff --git a/src/pad.c b/src/pad.c index a7427eb7..750e63b4 100644 --- a/src/pad.c +++ b/src/pad.c @@ -39,27 +39,18 @@ static FILE *pusb_pad_open_device( ) { FILE *f; - char path_devpad[(sizeof(mnt_point) + sizeof(opts->device_pad_directory) + 2)]; - char path_userpad[( - sizeof(mnt_point) + - sizeof(opts->device_pad_directory) + - sizeof(opts->hostname) + - sizeof(user) + - 7 - )]; - struct stat sb; - - memset(path_devpad, 0x00, sizeof(path_devpad)); - memset(path_userpad, 0x00, sizeof(path_userpad)); + char path_devpad[1024*5]; + char path_userpad[1024*5]; + struct stat sb; snprintf(path_devpad, sizeof(path_devpad), "%s/%s", mnt_point, opts->device_pad_directory); if (stat(path_devpad, &sb) != 0) { - log_debug("Directory %s does not exist, creating one.\n", path_devpad); + log_debug("Directory %s does not exist, creating it.\n", path_devpad); if (mkdir(path_devpad, S_IRUSR | S_IWUSR | S_IXUSR) != 0) { log_debug("Unable to create directory %s: %s\n", path_devpad, strerror(errno)); - return (NULL); + return NULL; } } @@ -76,7 +67,7 @@ static FILE *pusb_pad_open_device( if (!f) { log_debug("Cannot open device file: %s\n", strerror(errno)); - return (NULL); + return NULL; } return (f); } @@ -96,11 +87,10 @@ static FILE *pusb_pad_open_system( if (!(user_ent = getpwnam(user)) || !(user_ent->pw_dir)) { log_error("Unable to retrieve information for user \"%s\": %s\n", user, strerror(errno)); - return (0); + return 0; } - char path[(sizeof(user_ent->pw_dir) + sizeof(opts->system_pad_directory) + sizeof(device_name) + 1)]; - memset(path, 0x00, sizeof(path)); + char path[1024*5]; snprintf( path, sizeof(path), @@ -114,10 +104,10 @@ static FILE *pusb_pad_open_system( if (mkdir(path, S_IRUSR | S_IWUSR | S_IXUSR) != 0) { log_debug("Unable to create directory %s: %s\n", path, strerror(errno)); - return (NULL); + return NULL; } - if(chown(path, user_ent->pw_uid, user_ent->pw_gid) != 0) + if (chown(path, user_ent->pw_uid, user_ent->pw_gid) != 0) { log_error("Unable to chown directory %s: %s\n", path, strerror(errno)); } @@ -125,10 +115,10 @@ static FILE *pusb_pad_open_system( chmod(path, S_IRUSR | S_IWUSR | S_IXUSR); } /* change slashes in device name to underscores */ - snprintf(device_name, sizeof(opts->device.name), "%s", opts->device.name); - while(*device_name_ptr) + snprintf(device_name, sizeof(device_name), "%s", opts->device.name); + while (*device_name_ptr) { - if('/' == *device_name_ptr) + if ('/' == *device_name_ptr) { *device_name_ptr = '_'; } @@ -136,7 +126,6 @@ static FILE *pusb_pad_open_system( device_name_ptr++; } - memset(path, 0x00, sizeof(path)); snprintf( path, sizeof(path), @@ -149,9 +138,9 @@ static FILE *pusb_pad_open_system( if (!f) { log_debug("Cannot open system file: %s\n", strerror(errno)); - return (NULL); + return NULL; } - return (f); + return f; } static int pusb_pad_protect(const char *user, int fd) @@ -162,19 +151,19 @@ static int pusb_pad_protect(const char *user, int fd) if (!(user_ent = getpwnam(user))) { log_error("Unable to retrieve information for user \"%s\": %s\n", user, strerror(errno)); - return (0); + return 0; } if (fchown(fd, user_ent->pw_uid, user_ent->pw_gid) == -1) { log_debug("Unable to change owner of the pad: %s\n", strerror(errno)); - return (0); + return 0; } if (fchmod(fd, S_IRUSR | S_IWUSR) == -1) { log_debug("Unable to change mode of the pad: %s\n", strerror(errno)); - return (0); + return 0; } - return (1); + return 1; } static int pusb_pad_should_update(t_pusb_options *opts, const char *user) @@ -188,19 +177,19 @@ static int pusb_pad_should_update(t_pusb_options *opts, const char *user) if (!(f_system = pusb_pad_open_system(opts, user, "r"))) { log_debug("Unable to open system pad, pads must be generated.\n"); - return (1); + return 1; } if (fstat(fileno(f_system), &st) == -1) { fclose(f_system); - return (1); + return 1; } fclose(f_system); if (time(&now) == ((time_t)-1)) { log_error("Unable to fetch current time.\n"); - return (1); + return 1; } delta = now - st.st_mtime; @@ -208,14 +197,14 @@ static int pusb_pad_should_update(t_pusb_options *opts, const char *user) if (delta > opts->pad_expiration) { log_debug("Pads expired %u seconds ago, updating...\n", delta - opts->pad_expiration); - return (1); + return 1; } else { log_debug("Pads were generated %u seconds ago, not updating.\n", delta); - return (0); + return 0; } - return (1); + return 1; } static int pusb_pad_update( @@ -257,7 +246,7 @@ static int pusb_pad_update( * See https://crypto.stackexchange.com/a/35032 */ devrandom = open("/dev/random", O_RDONLY); - if (devrandom < 0 || read(devrandom, &seed, sizeof seed) != sizeof seed) + if (devrandom < 0 || read(devrandom, &seed, sizeof seed) != sizeof seed) { log_debug("/dev/random seeding failed...\n"); seed = getpid() * time(NULL); /* low-entropy fallback */ @@ -317,32 +306,32 @@ static int pusb_pad_compare( if (!(f_system = pusb_pad_open_system(opts, user, "r"))) { - return (1); + return 1; } if (!(f_device = pusb_pad_open_device(opts, volume, user, "r"))) { fclose(f_system); - return (0); + return 0; } log_debug("Loading device pad...\n"); bytes_read = fread(magic_device, sizeof(char), sizeof(magic_device), f_device); - if (!bytes_read) + if (!bytes_read) { log_error("Can't read device pad!\n"); fclose(f_system); fclose(f_device); - return (0); + return 0; } log_debug("Loading system pad...\n"); bytes_read = fread(magic_system, sizeof(char), sizeof(magic_system), f_system); - if (!bytes_read) + if (!bytes_read) { log_error("Can't read system pad!\n"); fclose(f_system); fclose(f_device); - return (0); + return 0; } retval = memcmp(magic_system, magic_device, sizeof(magic_system)); @@ -369,7 +358,7 @@ int pusb_pad_check( volume = pusb_volume_get(opts, udisks); if (!volume) { - return (0); + return 0; } retval = pusb_pad_compare(opts, volume->mount_point, user); @@ -386,5 +375,5 @@ int pusb_pad_check( } pusb_volume_destroy(volume); - return (retval); + return retval; } diff --git a/src/pam.c b/src/pam.c index f78b74f2..1cba9169 100644 --- a/src/pam.c +++ b/src/pam.c @@ -18,6 +18,7 @@ #define PAM_SM_AUTH #include #include +#include #include "version.h" #include "conf.h" @@ -34,25 +35,26 @@ int pam_sm_authenticate( ) { t_pusb_options opts; - const char *service; - const char *user; - const char *rhost; + const char *service = NULL; + const char *user = NULL; + const char *rhost = NULL; char *conf_file = PUSB_CONF_FILE; int retval; + memset(&opts, 0, sizeof(t_pusb_options)); // Initialize opts pusb_log_init(&opts); - retval = pam_get_item(pamh, PAM_SERVICE, (const void **)(const void *)&service); + retval = pam_get_item(pamh, PAM_SERVICE, (const void **)&service); if (retval != PAM_SUCCESS) { log_error("Unable to retrieve the PAM service name.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || !user || !*user) { log_error("Unable to retrieve the PAM user name.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (argc > 1 && !strcmp(argv[0], "-c")) @@ -62,32 +64,32 @@ int pam_sm_authenticate( if (!pusb_conf_init(&opts)) { - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (!pusb_conf_parse(conf_file, &opts, user, service)) { - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (!opts.enable) { log_debug("Not enabled, exiting...\n"); - return (PAM_IGNORE); + return PAM_IGNORE; } - if (opts.deny_remote) + if (opts.deny_remote) { - retval = pam_get_item(pamh, PAM_RHOST, (const void **)(const void *)&rhost); + retval = pam_get_item(pamh, PAM_RHOST, (const void **)&rhost); if (retval != PAM_SUCCESS) { log_error("Unable to retrieve PAM_RHOST.\n"); - return (PAM_AUTH_ERR); - } - else if (rhost != NULL && strcmp(rhost, "") != 0) + return PAM_AUTH_ERR; + } + else if (rhost != NULL && *rhost != '\0') { log_debug("RHOST is set (%s), must be a remote request - disabling myself for this request!\n", rhost); - return (PAM_IGNORE); + return PAM_IGNORE; } } @@ -96,15 +98,15 @@ int pam_sm_authenticate( if (pusb_local_login(&opts, user, service) != 1) { log_error("Access denied.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (pusb_device_check(&opts, user)) { log_info("Access granted.\n"); - return (PAM_SUCCESS); + return PAM_SUCCESS; } log_error("Access denied.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } PAM_EXTERN @@ -115,7 +117,7 @@ int pam_sm_setcred( const char **argv ) { - return (PAM_SUCCESS); + return PAM_SUCCESS; } PAM_EXTERN @@ -127,25 +129,26 @@ int pam_sm_acct_mgmt( ) { t_pusb_options opts; - const char *service; - const char *user; - const char *rhost; + const char *service = NULL; + const char *user = NULL; + const char *rhost = NULL; char *conf_file = PUSB_CONF_FILE; int retval; + memset(&opts, 0, sizeof(t_pusb_options)); // Initialize opts pusb_log_init(&opts); - retval = pam_get_item(pamh, PAM_SERVICE, (const void **)(const void *)&service); + retval = pam_get_item(pamh, PAM_SERVICE, (const void **)&service); if (retval != PAM_SUCCESS) { log_error("Unable to retrieve the PAM service name.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || !user || !*user) { log_error("Unable to retrieve the PAM user name.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (argc > 1 && !strcmp(argv[0], "-c")) @@ -155,32 +158,32 @@ int pam_sm_acct_mgmt( if (!pusb_conf_init(&opts)) { - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (!pusb_conf_parse(conf_file, &opts, user, service)) { - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (!opts.enable) { log_debug("Not enabled, exiting...\n"); - return (PAM_IGNORE); + return PAM_IGNORE; } - if (opts.deny_remote) + if (opts.deny_remote) { - retval = pam_get_item(pamh, PAM_RHOST, (const void **)(const void *)&rhost); + retval = pam_get_item(pamh, PAM_RHOST, (const void **)&rhost); if (retval != PAM_SUCCESS) { log_error("Unable to retrieve PAM_RHOST.\n"); - return (PAM_AUTH_ERR); - } - else if (rhost != NULL) + return PAM_AUTH_ERR; + } + else if (rhost != NULL && *rhost != '\0') { log_debug("RHOST is set (%s), must be a remote request - disabling myself for this request!\n", rhost); - return (PAM_IGNORE); + return PAM_IGNORE; } } @@ -190,15 +193,15 @@ int pam_sm_acct_mgmt( if (pusb_local_login(&opts, user, service) != 1) { log_error("Access denied.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } if (pusb_device_check(&opts, user)) { log_info("Access granted.\n"); - return (PAM_SUCCESS); + return PAM_SUCCESS; } log_error("Access denied.\n"); - return (PAM_AUTH_ERR); + return PAM_AUTH_ERR; } diff --git a/src/process.c b/src/process.c index e204360b..1de0fa7b 100644 --- a/src/process.c +++ b/src/process.c @@ -1,54 +1,51 @@ /** * Source: https://gist.github.com/fclairamb/a16a4237c46440bdb172 * Modifications: removed main(), added header file - * + * * Copyright 2014 Florent Clairambault (@fclairamb) - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in + * + * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. */ #include #include #include +#include #include "process.h" #include "mem.h" /** * Get a process name from its PID. * @param pid PID of the process - * @param name Name of the process + * @param name Buffer to store the process name * * Source: https://stackoverflow.com/questions/15545341/process-name-from-its-pid-in-linux */ -void pusb_get_process_name(const pid_t pid, char * name) +void pusb_get_process_name(const pid_t pid, char *name, size_t name_len) { char procfile[BUFSIZ]; sprintf(procfile, "/proc/%d/cmdline", pid); FILE* f = fopen(procfile, "r"); - if (f) + if (f) { - size_t size; - size = fread(name, sizeof (char), sizeof (procfile), f); - if (size > 0 && '\n' == name[size - 1]) - { - name[size - 1] = '\0'; - } + size_t size = fread(name, sizeof(char), name_len - 1, f); + name[size] = '\0'; // Ensure null-termination fclose(f); } } @@ -60,21 +57,20 @@ void pusb_get_process_name(const pid_t pid, char * name) * * Note: init is 1 and it has a parent id of 0. */ -void pusb_get_process_parent_id(const pid_t pid, pid_t * ppid) +void pusb_get_process_parent_id(const pid_t pid, pid_t *ppid) { char buffer[BUFSIZ]; sprintf(buffer, "/proc/%d/stat", pid); FILE* fp = fopen(buffer, "r"); - if (fp) + if (fp) { - size_t size = fread(buffer, sizeof (char), sizeof (buffer), fp); - if (size > 0) + if (fgets(buffer, sizeof(buffer), fp) != NULL) { // See: https://man7.org/linux/man-pages/man5/proc.5.html section /proc/[pid]/stat strtok(buffer, " "); // (1) pid %d strtok(NULL, " "); // (2) comm %s strtok(NULL, " "); // (3) state %c - char * s_ppid = strtok(NULL, " "); // (4) ppid %d + char *s_ppid = strtok(NULL, " "); // (4) ppid %d *ppid = atoi(s_ppid); } fclose(fp); @@ -97,17 +93,19 @@ void pusb_get_process_parent_id(const pid_t pid, pid_t * ppid) char *pusb_get_process_envvar(pid_t pid, char *var) { char buffer[BUFSIZ]; - char* output = xmalloc(BUFSIZ); + char *output = NULL; sprintf(buffer, "/proc/%d/environ", pid); FILE* fp = fopen(buffer, "r"); - if (fp) + if (fp) { - size_t size = fread(buffer, sizeof (char), sizeof (buffer), fp); + size_t size = fread(buffer, sizeof(char), sizeof(buffer) - 1, fp); + buffer[size] = '\0'; // Ensure null-termination fclose(fp); - for (int i = 0 ; i < size; i++) + + for (size_t i = 0; i < size; i++) { - if (!buffer[i] && i != size) buffer[i] = '#'; // replace \0 with "#" since strtok uses \0 internally + if (buffer[i] == '\0') buffer[i] = '#'; // replace \0 with "#" since strtok uses \0 internally } if (size > 0) @@ -115,17 +113,16 @@ char *pusb_get_process_envvar(pid_t pid, char *var) char* variable_content = strtok(buffer, "#"); while (variable_content != NULL) { - if (strncmp(var, variable_content, strlen(var)) == 0) + if (strncmp(var, variable_content, strlen(var)) == 0 && variable_content[strlen(var)] == '=') { - return output; + output = strdup(variable_content + strlen(var) + 1); // Allocate memory and copy the content + break; } variable_content = strtok(NULL, "#"); - strcpy(output, variable_content + (strlen(var) +1)); // cut var= and set output string } } } - xfree(output); - return NULL; + return output; } \ No newline at end of file diff --git a/src/process.h b/src/process.h index d9e31d50..0b411792 100644 --- a/src/process.h +++ b/src/process.h @@ -25,7 +25,7 @@ #include -void pusb_get_process_name(const pid_t pid, char * name); +void pusb_get_process_name(const pid_t pid, char * name, size_t name_len); void pusb_get_process_parent_id(const pid_t pid, pid_t * ppid); diff --git a/src/tmux.c b/src/tmux.c index 9a6b75da..25a4bad2 100644 --- a/src/tmux.c +++ b/src/tmux.c @@ -26,12 +26,12 @@ char *pusb_tmux_get_client_tty(pid_t env_pid) { char *tmux_details = getenv("TMUX"); - if (tmux_details == NULL) + if (tmux_details == NULL) { log_debug(" No TMUX env var, checking parent process in case this is a sudo request\n"); tmux_details = pusb_get_process_envvar(env_pid, "TMUX"); - if (tmux_details == NULL) + if (tmux_details == NULL) { return NULL; } @@ -50,29 +50,36 @@ char *pusb_tmux_get_client_tty(pid_t env_pid) char buf[BUFSIZ]; FILE *fp; - if ((fp = popen(get_tmux_session_details_cmd, "r")) == NULL) + if ((fp = popen(get_tmux_session_details_cmd, "r")) == NULL) { log_error("tmux detected, but couldn't get session details. Denying since remote check impossible without it!\n"); return (0); } char *tmux_client_tty = NULL; - if (fgets(buf, BUFSIZ, fp) != NULL) + if (fgets(buf, BUFSIZ, fp) != NULL) { tmux_client_tty = strtok(buf, ":"); tmux_client_tty += 5; // cut "/dev/" log_debug(" Got tmux_client_tty: %s\n", tmux_client_tty); - if (pclose(fp)) + if (pclose(fp)) { log_debug(" Closing pipe for 'tmux list-clients' failed, this is quite a wtf...\n"); } - return tmux_client_tty; - } - else + char *result = xmalloc(strlen(tmux_client_tty) + 1); + if (result == NULL) { + log_error("Memory allocation failed\n"); + return 0; + } + strcpy(result, tmux_client_tty); + return result; + } + else { log_error("tmux detected, but couldn't get client details. Denying since remote check impossible without it!\n"); + pclose(fp); return (0); } } @@ -95,52 +102,55 @@ int pusb_tmux_has_remote_clients(const char* username) "(.+)([0-9A-Fa-f]{1,4}):([0-9A-Fa-f]{1,4}):([0-9A-Fa-f]{1,4}):([0-9A-Fa-f]{1,4})(.+)tmux(.+)" // v6 }; // ... yes, these allow invalid addresses. No, I don't care. This isn't about validation but detecting remote access. Good enough ¯\_(ツ)_/¯ - for (int i = 0; i <= 1; i++) + for (int i = 0; i <= 1; i++) { log_debug(" Checking for IPv%d connections...\n", (4 + (i * 2))); if ((fp = popen("LC_ALL=c; w", "r")) == NULL) { log_error("tmux detected, but couldn't get `w`. Denying since remote check for tmux impossible without it!\n"); - return (-1); + return -1; } - while (fgets(buf, BUFSIZ, fp) != NULL) + while (fgets(buf, BUFSIZ, fp) != NULL) { - sprintf(regex_raw, "%s%s", username, regex_tpl[i]); + snprintf(regex_raw, BUFSIZ, "%s%s", username, regex_tpl[i]); status = regcomp(®ex, regex_raw, REG_EXTENDED); - if (status) + if (status) { log_debug(" Couldn't compile regex!\n"); regfree(®ex); - return (-1); + pclose(fp); + return -1; } status = regexec(®ex, buf, 0, NULL, 0); - if (!status) + if (!status) { log_error("tmux detected and at least one remote client is connected to the session, denying!\n"); regfree(®ex); + pclose(fp); return 1; } - else if (status != REG_NOMATCH) + else if (status != REG_NOMATCH) { regerror(status, ®ex, msgbuf, sizeof(msgbuf)); log_debug(" Regex match failed: %s\n", msgbuf); regfree(®ex); + pclose(fp); return -1; } regfree(®ex); } - if (pclose(fp)) + if (pclose(fp)) { log_debug(" Closing pipe for 'w' failed, this is quite a wtf...\n"); } } - // If we would have detected a remote access we would have returned by now. Safe to return 0 now + // If we had detected a remote access we would have returned by now. Safe to return 0 now return 0; } \ No newline at end of file diff --git a/src/volume.c b/src/volume.c index 56401fa2..1fe8023b 100644 --- a/src/volume.c +++ b/src/volume.c @@ -61,9 +61,16 @@ static int pusb_volume_mount(t_pusb_volume *volume) { g_main_context_iteration(NULL, FALSE); mount_points = udisks_filesystem_get_mount_points(volume->filesystem); - volume->mount_point = xstrdup(*mount_points); - retval = 1; - log_debug("Device %s mounted in between our probe and mount.\n", volume->device); + if (mount_points && *mount_points) + { + volume->mount_point = xstrdup(*mount_points); + retval = 1; + log_debug("Device %s mounted in between our probe and mount.\n", volume->device); + } + else + { + log_error("Device %s mounted in between our probe and mount, but could not get the list mount points.\n", volume->device); + } } else { @@ -166,7 +173,7 @@ t_pusb_volume *pusb_volume_get(t_pusb_options *opts, UDisksClient *udisks) return (volume); } - if(!pusb_volume_mount(volume)) + if (!pusb_volume_mount(volume)) { pusb_volume_destroy(volume); return (NULL); @@ -177,7 +184,7 @@ t_pusb_volume *pusb_volume_get(t_pusb_options *opts, UDisksClient *udisks) void pusb_volume_destroy(t_pusb_volume *volume) { - GVariantBuilder builder; + GVariantBuilder builder; GVariant *options; int ret; @@ -207,3 +214,4 @@ void pusb_volume_destroy(t_pusb_volume *volume) xfree(volume->mount_point); xfree(volume); } +