From 55a8a26a24ef9590065da1e8fb8cdee5e81d36cc Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 6 Mar 2025 11:19:00 +0100 Subject: [PATCH 1/5] Attempt to open file later to prevent potential resource leak --- src/command/cmd_funcs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 63e16415..51439237 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -4917,8 +4917,6 @@ cmd_sendfile(ProfWin* window, const char* const command, gchar** args) goto out; } - FILE* fh = fdopen(fd, "rb"); - gboolean omemo_enabled = FALSE; gboolean sendfile_enabled = TRUE; @@ -4952,6 +4950,8 @@ cmd_sendfile(ProfWin* window, const char* const command, gchar** args) goto out; } + FILE* fh = fdopen(fd, "rb"); + if (omemo_enabled) { #ifdef HAVE_OMEMO char* err = NULL; From 43bbb16e8e00869937eea940741f1aa85f705dc5 Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 6 Mar 2025 11:19:28 +0100 Subject: [PATCH 2/5] Close file in case of error --- src/tools/aesgcm_download.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/aesgcm_download.c b/src/tools/aesgcm_download.c index 752c50ea..7fdc7174 100644 --- a/src/tools/aesgcm_download.c +++ b/src/tools/aesgcm_download.c @@ -122,6 +122,9 @@ aesgcm_file_get(void* userdata) "temporary file at '%s' for reading (%s).", aesgcm_dl->url, tmpname, g_strerror(errno)); + if (fclose(outfh) == EOF) { + cons_show_error(g_strerror(errno)); + } return NULL; } From b5f0cc0bd4d3e0c7245b8c1f6361584248db371a Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 6 Mar 2025 14:27:13 +0100 Subject: [PATCH 3/5] Fix more potential leaks in aesgcm_download --- .github/workflows/main.yml | 4 ++-- src/common.c | 30 ++++++++++++++++++++++++++++++ src/common.h | 8 ++++++++ src/tools/aesgcm_download.c | 27 ++++++--------------------- 4 files changed, 46 insertions(+), 23 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 406b58fd..1502301b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -31,10 +31,10 @@ jobs: # if this check fails, you have to update the number of auto types known and the list of auto types in the check below - name: Check auto types are up-to-date run: | - [[ "$(find src -type f -name '*.[ch]' -exec awk '/^#define auto_[\W]*/ {print $2}' '{}' \; | sort -u | wc -l)" == "6" ]] || exit -1 + [[ "$(find src -type f -name '*.[ch]' -exec awk '/^#define auto_[\W]*/ {print $2}' '{}' \; | sort -u | wc -l)" == "8" ]] || exit -1 - name: Check auto types are initialized run: | - grep -P 'auto_(char|gchar|gcharv|guchar|jid|sqlite)[\w *]*;$' -r src && exit -1 || true + grep -P 'auto_(char|gchar|gcharv|guchar|jid|sqlite|gfd|FILE)[\w *]*;$' -r src && exit -1 || true - name: Run clang-format uses: jidicula/clang-format-action@v4.11.0 with: diff --git a/src/common.c b/src/common.c index 97447381..85bc60e0 100644 --- a/src/common.c +++ b/src/common.c @@ -130,6 +130,36 @@ auto_free_char(char** str) free(*str); } +/** + * Closes the file descriptor. + * + * @param fd file descriptor handle. If NULL, no action is taken. + */ +void +auto_close_gfd(gint* fd) +{ + if (fd == NULL) + return; + + if (close(*fd) == EOF) + log_error(g_strerror(errno)); +} + +/** + * Closes file stream + * + * @param fd file descriptor handle opened with fopen. If NULL, no action is taken. + */ +void +auto_close_FILE(FILE** fd) +{ + if (fd == NULL) + return; + + if (fclose(*fd) == EOF) + log_error(g_strerror(errno)); +} + static gboolean _load_keyfile(prof_keyfile_t* keyfile) { diff --git a/src/common.h b/src/common.h index 5996c5d5..9ebafa66 100644 --- a/src/common.h +++ b/src/common.h @@ -104,6 +104,14 @@ void auto_free_guchar(guchar** str); */ #define auto_guchar __attribute__((__cleanup__(auto_free_guchar))) +#define auto_gfd __attribute__((__cleanup__(auto_close_gfd))) + +void auto_close_gfd(gint* fd); + +#define auto_FILE __attribute__((__cleanup__(auto_close_FILE))) + +void auto_close_FILE(FILE** fd); + #if defined(__OpenBSD__) #define STR_MAYBE_NULL(p) (p) ?: "(null)" #else diff --git a/src/tools/aesgcm_download.c b/src/tools/aesgcm_download.c index 7fdc7174..04e837cc 100644 --- a/src/tools/aesgcm_download.c +++ b/src/tools/aesgcm_download.c @@ -64,8 +64,8 @@ aesgcm_file_get(void* userdata) { AESGCMDownload* aesgcm_dl = (AESGCMDownload*)userdata; - char* https_url = NULL; - char* fragment = NULL; + auto_char char* https_url = NULL; + auto_char char* fragment = NULL; // Convert the aesgcm:// URL to a https:// URL and extract the encoded key // and tag stored in the URL fragment. @@ -79,8 +79,8 @@ aesgcm_file_get(void* userdata) // Create a temporary file used for storing the ciphertext that is to be // retrieved from the https:// URL. - gchar* tmpname = NULL; - gint tmpfd; + auto_gchar char* tmpname = NULL; + auto_gfd gint tmpfd = 0; if ((tmpfd = g_file_open_tmp("profanity.XXXXXX", &tmpname, NULL)) == -1) { http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, "Downloading '%s' failed: Unable to create " @@ -91,7 +91,7 @@ aesgcm_file_get(void* userdata) } // Open the target file for storing the cleartext. - FILE* outfh = fopen(aesgcm_dl->filename, "wb"); + auto_FILE FILE* outfh = fopen(aesgcm_dl->filename, "wb"); if (outfh == NULL) { http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, "Downloading '%s' failed: Unable to open " @@ -115,16 +115,13 @@ aesgcm_file_get(void* userdata) http_file_get(http_dl); // TODO(wstrm): Verify result. - FILE* tmpfh = fopen(tmpname, "rb"); + auto_FILE FILE* tmpfh = fopen(tmpname, "rb"); if (tmpfh == NULL) { http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->id, "Downloading '%s' failed: Unable to open " "temporary file at '%s' for reading (%s).", aesgcm_dl->url, tmpname, g_strerror(errno)); - if (fclose(outfh) == EOF) { - cons_show_error(g_strerror(errno)); - } return NULL; } @@ -132,11 +129,6 @@ aesgcm_file_get(void* userdata) crypt_res = omemo_decrypt_file(tmpfh, outfh, http_dl->bytes_received, fragment); - if (fclose(tmpfh) == EOF) { - cons_show_error(g_strerror(errno)); - } - - close(tmpfd); remove(tmpname); g_free(tmpname); @@ -147,13 +139,6 @@ aesgcm_file_get(void* userdata) https_url, gcry_strerror(crypt_res)); } - if (fclose(outfh) == EOF) { - cons_show_error(g_strerror(errno)); - } - - free(https_url); - free(fragment); - if (aesgcm_dl->cmd_template != NULL) { gchar** argv = format_call_external_argv(aesgcm_dl->cmd_template, aesgcm_dl->filename, From 71b7d97dcb97d7ffadcd3d80e5a573456fe56b3c Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 6 Mar 2025 14:29:20 +0100 Subject: [PATCH 4/5] Get rid of unused define --- src/tools/aesgcm_download.c | 2 -- src/tools/plugin_download.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/tools/aesgcm_download.c b/src/tools/aesgcm_download.c index 04e837cc..6aa56013 100644 --- a/src/tools/aesgcm_download.c +++ b/src/tools/aesgcm_download.c @@ -57,8 +57,6 @@ #include "ui/window.h" #include "common.h" -#define FALLBACK_MSG "" - void* aesgcm_file_get(void* userdata) { diff --git a/src/tools/plugin_download.c b/src/tools/plugin_download.c index 95b50474..99176ec2 100644 --- a/src/tools/plugin_download.c +++ b/src/tools/plugin_download.c @@ -57,8 +57,6 @@ #include "ui/window.h" #include "common.h" -#define FALLBACK_MSG "" - void* plugin_download_install(void* userdata) { From bf24d79990b6f46607aeaa375d20d9913f4b2f0d Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 6 Mar 2025 14:38:35 +0100 Subject: [PATCH 5/5] Add plist to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7b04d430..37a7838b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ compile_commands.json .project .settings/ .vscode/ +*.plist/ # autotools .libs/