From 764f8215a9dad09b93832fdbb0c87eb1b0e6e773 Mon Sep 17 00:00:00 2001 From: Lukas Waymann Date: Sun, 12 Aug 2018 07:22:14 +0000 Subject: [PATCH 1/2] Fix `/save` replacing symlinks with regular files A side-effect of 8deb618 is that `/save` may replace configuration files that are symlinks with regular files. Fix this by resolving all symlinks before renaming the temporary file. --- src/lib-config/write.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/lib-config/write.c b/src/lib-config/write.c index ab2c6975..d325fc52 100644 --- a/src/lib-config/write.c +++ b/src/lib-config/write.c @@ -304,6 +304,7 @@ int config_write(CONFIG_REC *rec, const char *fname, int create_mode) int save_errno; char *tmp_name; const char *dest_name; + char *real_dest = NULL; g_return_val_if_fail(rec != NULL, -1); g_return_val_if_fail(fname != NULL || rec->fname != NULL, -1); @@ -349,7 +350,15 @@ int config_write(CONFIG_REC *rec, const char *fname, int create_mode) g_io_channel_unref(rec->handle); rec->handle = NULL; - if (rename(tmp_name, dest_name) == -1) { + /* expand all symlinks; else we may replace a symlink with a regular file */ + real_dest = realpath(dest_name, NULL); + if (real_dest == NULL) { + unlink(tmp_name); + config_error(rec, g_strerror(errno)); + goto out; + } + + if (rename(tmp_name, real_dest) == -1) { unlink(tmp_name); config_error(rec, g_strerror(errno)); goto out; @@ -362,6 +371,7 @@ out: } g_free(tmp_name); + g_free(real_dest); return ret; } From 7d3eb47ab51a93defb267912704e84b3241b4db6 Mon Sep 17 00:00:00 2001 From: Lukas Waymann Date: Sun, 12 Aug 2018 10:55:47 +0000 Subject: [PATCH 2/2] Fix potential `rename(3)` across file systems Make sure the temporary file in the `config_write` function is created on the same file system as the file we `rename` it to later. --- src/lib-config/write.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/lib-config/write.c b/src/lib-config/write.c index d325fc52..255d79c0 100644 --- a/src/lib-config/write.c +++ b/src/lib-config/write.c @@ -302,15 +302,21 @@ int config_write(CONFIG_REC *rec, const char *fname, int create_mode) int ret; int fd; int save_errno; - char *tmp_name; - const char *dest_name; - char *real_dest = NULL; + char *tmp_name = NULL; + char *dest_name = NULL; g_return_val_if_fail(rec != NULL, -1); g_return_val_if_fail(fname != NULL || rec->fname != NULL, -1); g_return_val_if_fail(create_mode != -1 || rec->create_mode != -1, -1); - dest_name = fname != NULL ? fname : rec->fname; + /* expand all symlinks; else we may replace a symlink with a regular file */ + dest_name = realpath(fname != NULL ? fname : rec->fname, NULL); + if (dest_name == NULL) { + config_error(rec, g_strerror(errno)); + ret = -1; + goto out; + } + tmp_name = g_strdup_printf("%s.XXXXXX", dest_name); fd = g_mkstemp_full(tmp_name, @@ -350,15 +356,7 @@ int config_write(CONFIG_REC *rec, const char *fname, int create_mode) g_io_channel_unref(rec->handle); rec->handle = NULL; - /* expand all symlinks; else we may replace a symlink with a regular file */ - real_dest = realpath(dest_name, NULL); - if (real_dest == NULL) { - unlink(tmp_name); - config_error(rec, g_strerror(errno)); - goto out; - } - - if (rename(tmp_name, real_dest) == -1) { + if (rename(tmp_name, dest_name) == -1) { unlink(tmp_name); config_error(rec, g_strerror(errno)); goto out; @@ -371,7 +369,7 @@ out: } g_free(tmp_name); - g_free(real_dest); + g_free(dest_name); return ret; }