From fbb5e4651a646bb9556c43f26eca110a42274105 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 13 Dec 2006 18:01:45 +0100 Subject: [PATCH] Use m_temp instead of mutt_mktemp in compress code This patch remove the static set_path() function which was used only twice. in: - mutt_open_read_compressed() We just create a temporary file and open it, so this is more secure against symlink attacks and then just call decompression cmd normaly - mutt_open_append_compressed() Instead of creating the file if (cond), unlink() it if (!cond). Signed-off-by: Julien Danjou Signed-off-by: Pierre Habouzit --- lib-mx/compress.c | 101 ++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/lib-mx/compress.c b/lib-mx/compress.c index 044392e..9e44f34 100644 --- a/lib-mx/compress.c +++ b/lib-mx/compress.c @@ -45,7 +45,7 @@ static int mbox_lock_compressed (CONTEXT * ctx, FILE * fp, int excl, int retry) return 0; } - return (r); + return r; } static void mbox_unlock_compressed (CONTEXT * ctx, FILE * fp) @@ -60,7 +60,7 @@ static void mbox_unlock_compressed (CONTEXT * ctx, FILE * fp) static int is_new (const char *path) { - return (access (path, W_OK) != 0 && errno == ENOENT) ? 1 : 0; + return (access (path, W_OK) && errno == ENOENT); } static const char *find_compress_hook (int type, const char *path) @@ -113,25 +113,13 @@ static compress_info *set_compress_info (CONTEXT * ctx) return (ctx->cinfo = ci); } -static void set_path (CONTEXT * ctx) -{ - char tmppath[_POSIX_PATH_MAX]; - - /* Setup the right paths */ - ctx->realpath = ctx->path; - - /* Uncompress to /tmp */ - mutt_mktemp (tmppath); - ctx->path = p_dupstr(tmppath, m_strlen(tmppath)); -} - static int get_size (const char *path) { struct stat sb; if (stat (path, &sb) != 0) return 0; - return (sb.st_size); + return sb.st_size; } static const char * @@ -156,7 +144,7 @@ compresshook_format_str(char *dest, ssize_t destlen, snprintf (dest, destlen, tmp, ctx->path); break; } - return (src); + return src; } /* check that the command has both %f and %t @@ -181,9 +169,9 @@ int mutt_check_mailbox_compressed (CONTEXT * ctx) p_delete(&ctx->realpath); mutt_error _("Mailbox was corrupted!"); - return (-1); + return -1; } - return (0); + return 0; } int mutt_open_read_compressed (CONTEXT * ctx) @@ -191,18 +179,34 @@ int mutt_open_read_compressed (CONTEXT * ctx) char *cmd; FILE *fp; int rc; + char tmppath[_POSIX_PATH_MAX]; + int tmpfd; compress_info *ci = set_compress_info (ctx); if (!ci->open) { ctx->magic = 0; p_delete(&ctx->cinfo); - return (-1); + return -1; } if (!ci->close || access (ctx->path, W_OK) != 0) ctx->readonly = 1; - set_path (ctx); + /* Setup the right paths */ + ctx->realpath = ctx->path; + + /* Uncompress to /tmp */ + tmpfd = m_tempfd(tmppath, sizeof(tmppath), NONULL(Tempdir), NULL); + /* If we cannot open tempfile, that means the file already exists (!?) + * or we are following a symlink, which is bad and insecure. + */ + if(!tmpfd) { + return -1; + } + close(tmpfd); + + ctx->path = p_dupstr(tmppath, m_strlen(tmppath)); + ctx->cinfo->size = get_size(ctx->realpath); if (!ctx->quiet) @@ -210,12 +214,12 @@ int mutt_open_read_compressed (CONTEXT * ctx) cmd = get_compression_cmd (ci->open, ctx); if (cmd == NULL) - return (-1); + return -1; if ((fp = fopen (ctx->realpath, "r")) == NULL) { mutt_perror (ctx->realpath); p_delete(&cmd); - return (-1); + return -1; } mutt_block_signals (); if (mbox_lock_compressed (ctx, fp, 0, 1) == -1) { @@ -224,7 +228,7 @@ int mutt_open_read_compressed (CONTEXT * ctx) mutt_error _("Unable to lock mailbox!"); p_delete(&cmd); - return (-1); + return -1; } endwin (); @@ -244,14 +248,14 @@ int mutt_open_read_compressed (CONTEXT * ctx) } p_delete(&cmd); if (rc) - return (-1); + return -1; if (mutt_check_mailbox_compressed (ctx)) - return (-1); + return -1; ctx->magic = mx_get_magic (ctx->path); - return (0); + return 0; } static void restore_path (CONTEXT * ctx) @@ -271,27 +275,36 @@ int mutt_open_append_compressed (CONTEXT * ctx) { FILE *fh; compress_info *ci = set_compress_info (ctx); + char tmppath[_POSIX_PATH_MAX]; if (!get_append_command (ctx->path, ctx)) { if (ci->open && ci->close) - return (mutt_open_read_compressed (ctx)); + return mutt_open_read_compressed (ctx); ctx->magic = 0; p_delete(&ctx->cinfo); - return (-1); + return -1; } - set_path (ctx); + /* Setup the right paths */ + ctx->realpath = ctx->path; + + /* Uncompress to /tmp */ + fh = m_tempfile(tmppath, sizeof(tmppath), NONULL(Tempdir), NULL); + m_fclose(&fh); + + ctx->path = p_dupstr(tmppath, m_strlen(tmppath)); ctx->magic = DefaultMagic; - if (!is_new (ctx->realpath)) - if (ctx->magic == M_MBOX || ctx->magic == M_MMDF) - if ((fh = safe_fopen (ctx->path, "w"))) - m_fclose(&fh); + if (is_new (ctx->realpath) || + (ctx->magic != M_MBOX && + ctx->magic != M_MMDF)) + unlink(tmppath); + /* No error checking - the parent function will catch it */ - return (0); + return 0; } /* close a compressed mailbox */ @@ -323,12 +336,12 @@ int mutt_sync_compressed (CONTEXT * ctx) cmd = get_compression_cmd (ctx->cinfo->close, ctx); if (cmd == NULL) - return (-1); + return -1; if ((fp = fopen (ctx->realpath, "a")) == NULL) { mutt_perror (ctx->realpath); p_delete(&cmd); - return (-1); + return -1; } mutt_block_signals (); if (mbox_lock_compressed (ctx, fp, 1, 1) == -1) { @@ -339,7 +352,7 @@ int mutt_sync_compressed (CONTEXT * ctx) ctx->cinfo->size = get_size(ctx->realpath); p_delete(&cmd); - return (-1); + return -1; } endwin (); @@ -362,7 +375,7 @@ int mutt_sync_compressed (CONTEXT * ctx) ctx->cinfo->size = get_size(ctx->realpath); - return (rc); + return rc; } int mutt_slow_close_compressed (CONTEXT * ctx) @@ -377,7 +390,7 @@ int mutt_slow_close_compressed (CONTEXT * ctx) /* if we can not or should not append, we only have to remove the compressed info, because sync was already called */ mutt_fast_close_compressed (ctx); - return (0); + return 0; } m_fclose(&ctx->fp); @@ -391,12 +404,12 @@ int mutt_slow_close_compressed (CONTEXT * ctx) cmd = get_compression_cmd (append, ctx); if (cmd == NULL) - return (-1); + return -1; if ((fp = fopen (ctx->realpath, "a")) == NULL) { mutt_perror (ctx->realpath); p_delete(&cmd); - return (-1); + return -1; } mutt_block_signals (); if (mbox_lock_compressed (ctx, fp, 1, 1) == -1) { @@ -405,7 +418,7 @@ int mutt_slow_close_compressed (CONTEXT * ctx) mutt_error _("Unable to lock mailbox!"); p_delete(&cmd); - return (-1); + return -1; } endwin (); @@ -426,7 +439,7 @@ int mutt_slow_close_compressed (CONTEXT * ctx) mbox_unlock_compressed (ctx, fp); mutt_unblock_signals (); m_fclose(&fp); - return (-1); + return -1; } mbox_unlock_compressed (ctx, fp); @@ -437,7 +450,7 @@ int mutt_slow_close_compressed (CONTEXT * ctx) p_delete(&cmd); p_delete(&ctx->cinfo); - return (0); + return 0; } mx_t const compress_mx = { -- 2.20.1