/pidgin/main: 9a2250c4e9d1: Fix Coverity TOCTOU warnings
Tomasz Wasilczyk
twasilczyk at pidgin.im
Mon May 12 17:56:02 EDT 2014
Changeset: 9a2250c4e9d17b409133dda9a6074914094dcade
Author: Tomasz Wasilczyk <twasilczyk at pidgin.im>
Date: 2014-05-12 23:55 +0200
Branch: release-2.x.y
URL: https://hg.pidgin.im/pidgin/main/rev/9a2250c4e9d1
Description:
Fix Coverity TOCTOU warnings
diffstat:
libpurple/log.c | 25 +++++-----
libpurple/protocols/silc/util.c | 95 +++++++++-------------------------------
libpurple/util.c | 7 ++-
3 files changed, 42 insertions(+), 85 deletions(-)
diffs (213 lines):
diff --git a/libpurple/log.c b/libpurple/log.c
--- a/libpurple/log.c
+++ b/libpurple/log.c
@@ -1676,7 +1676,7 @@ static GList *old_logger_list(PurpleLogT
time_t log_last_modified;
FILE *index;
FILE *file;
- int index_fd;
+ int file_fd, index_fd;
char *index_tmp;
char buf[BUF_LONG];
struct tm tm;
@@ -1692,13 +1692,21 @@ static GList *old_logger_list(PurpleLogT
g_free(logfile);
- if (g_stat(purple_stringref_value(pathref), &st))
- {
+ file_fd = g_open(purple_stringref_value(pathref), 0, O_RDONLY);
+ if (file_fd == -1 || (file = fdopen(file_fd, "rb")) == NULL) {
+ purple_debug_error("log",
+ "Failed to open log file \"%s\" for reading: %s\n",
+ purple_stringref_value(pathref), g_strerror(errno));
purple_stringref_unref(pathref);
g_free(pathstr);
return NULL;
}
- else
+ if (fstat(file_fd, &st) == -1) {
+ purple_stringref_unref(pathref);
+ g_free(pathstr);
+ fclose(file);
+ return NULL;
+ } else
log_last_modified = st.st_mtime;
/* Change the .log extension to .idx */
@@ -1752,19 +1760,12 @@ static GList *old_logger_list(PurpleLogT
fclose(index);
purple_stringref_unref(pathref);
+ fclose(file);
return list;
}
}
}
- if (!(file = g_fopen(purple_stringref_value(pathref), "rb"))) {
- purple_debug_error("log", "Failed to open log file \"%s\" for reading: %s\n",
- purple_stringref_value(pathref), g_strerror(errno));
- purple_stringref_unref(pathref);
- g_free(pathstr);
- return NULL;
- }
-
index_tmp = g_strdup_printf("%s.XXXXXX", pathstr);
if ((index_fd = g_mkstemp(index_tmp)) == -1) {
purple_debug_error("log", "Failed to open index temp file: %s\n",
diff --git a/libpurple/protocols/silc/util.c b/libpurple/protocols/silc/util.c
--- a/libpurple/protocols/silc/util.c
+++ b/libpurple/protocols/silc/util.c
@@ -92,104 +92,55 @@ gboolean silcpurple_check_silc_dir(Purpl
g_snprintf(friendsfilename, sizeof(friendsfilename) - 1, "%s" G_DIR_SEPARATOR_S "friends",
silcpurple_silcdir());
+ if (pw->pw_uid != geteuid()) {
+ purple_debug_error("silc", "Couldn't create directories due to wrong uid!\n");
+ return FALSE;
+ }
+
/*
* Check ~/.silc directory
*/
+ if (g_mkdir(filename, 0755) != 0 && errno != EEXIST) {
+ purple_debug_error("silc", "Couldn't create '%s' directory\n", filename);
+ return FALSE;
+ }
+
+#ifndef _WIN32
if ((g_stat(filename, &st)) == -1) {
- /* If dir doesn't exist */
- if (errno == ENOENT) {
- if (pw->pw_uid == geteuid()) {
- if ((g_mkdir(filename, 0755)) == -1) {
- purple_debug_error("silc", "Couldn't create '%s' directory\n", filename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't create '%s' directory due to a wrong uid!\n",
- filename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't stat '%s' directory, error: %s\n", filename, g_strerror(errno));
- return FALSE;
- }
+ purple_debug_error("silc", "Couldn't stat '%s' directory, error: %s\n", filename, g_strerror(errno));
+ return FALSE;
} else {
-#ifndef _WIN32
/* Check the owner of the dir */
if (st.st_uid != 0 && st.st_uid != pw->pw_uid) {
purple_debug_error("silc", "You don't seem to own '%s' directory\n",
filename);
return FALSE;
}
+ }
#endif
- }
/*
* Check ~./silc/serverkeys directory
*/
- if ((g_stat(servfilename, &st)) == -1) {
- /* If dir doesn't exist */
- if (errno == ENOENT) {
- if (pw->pw_uid == geteuid()) {
- if ((g_mkdir(servfilename, 0755)) == -1) {
- purple_debug_error("silc", "Couldn't create '%s' directory\n", servfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't create '%s' directory due to a wrong uid!\n",
- servfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't stat '%s' directory, error: %s\n",
- servfilename, g_strerror(errno));
- return FALSE;
- }
+ if (g_mkdir(servfilename, 0755) != 0 && errno != EEXIST) {
+ purple_debug_error("silc", "Couldn't create '%s' directory\n", servfilename);
+ return FALSE;
}
/*
* Check ~./silc/clientkeys directory
*/
- if ((g_stat(clientfilename, &st)) == -1) {
- /* If dir doesn't exist */
- if (errno == ENOENT) {
- if (pw->pw_uid == geteuid()) {
- if ((g_mkdir(clientfilename, 0755)) == -1) {
- purple_debug_error("silc", "Couldn't create '%s' directory\n", clientfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't create '%s' directory due to a wrong uid!\n",
- clientfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't stat '%s' directory, error: %s\n",
- clientfilename, g_strerror(errno));
- return FALSE;
- }
+ if (g_mkdir(clientfilename, 0755) != 0 && errno != EEXIST) {
+ purple_debug_error("silc", "Couldn't create '%s' directory\n", clientfilename);
+ return FALSE;
}
/*
* Check ~./silc/friends directory
*/
- if ((g_stat(friendsfilename, &st)) == -1) {
- /* If dir doesn't exist */
- if (errno == ENOENT) {
- if (pw->pw_uid == geteuid()) {
- if ((g_mkdir(friendsfilename, 0755)) == -1) {
- purple_debug_error("silc", "Couldn't create '%s' directory\n", friendsfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't create '%s' directory due to a wrong uid!\n",
- friendsfilename);
- return FALSE;
- }
- } else {
- purple_debug_error("silc", "Couldn't stat '%s' directory, error: %s\n",
- friendsfilename, g_strerror(errno));
- return FALSE;
- }
+ if (g_mkdir(friendsfilename, 0755) != 0 && errno != EEXIST) {
+ purple_debug_error("silc", "Couldn't create '%s' directory\n", friendsfilename);
+ return FALSE;
}
/*
diff --git a/libpurple/util.c b/libpurple/util.c
--- a/libpurple/util.c
+++ b/libpurple/util.c
@@ -2696,7 +2696,11 @@ purple_util_write_data_to_file_absolute(
g_free(filename_temp);
return FALSE;
}
- /* Use stat to be absolutely sure. */
+#ifndef __COVERITY__
+ /* Use stat to be absolutely sure.
+ * It causes TOCTOU coverity warning (against g_rename below),
+ * but it's not a threat for us.
+ */
if ((g_stat(filename_temp, &st) == -1) || (st.st_size != real_size))
{
purple_debug_error("util", "Error writing data to file %s: "
@@ -2706,6 +2710,7 @@ purple_util_write_data_to_file_absolute(
g_free(filename_temp);
return FALSE;
}
+#endif /* __COVERITY__ */
/* Rename to the REAL name */
if (g_rename(filename_temp, filename_full) == -1)
More information about the Commits
mailing list