[Gaim-commits] SF.net SVN: gaim: [16061] trunk/src/protocols/silc/util.c

[<the...@us...>]

Revision: 16061
Author:   thekingant
Date:     2006-04-18 19:12:45 -0700 (Tue, 18 Apr 2006)
ViewCVS:  http://svn.sourceforge.net/gaim/?rev=16061&view=rev

Log Message:
-----------
silcgaim_check_silc_dir() checks to make sure the user's private
key has permission 0600.  If it doesn't, it chmod's the file.
Nathanael Hoyle pointed out the totally absurd scenario where, if
Gaim is suid root, someone could replace the private key with
something else between the fstat and the chmod so that the file
permissions are changed on a file that the user wouldn't otherwise
have access to.  He also suggested a fix along the lines of this
one.

Ethan said this still isn't totally safe, but it should be a little
better, and I don't really care anyway because you'd have to be a
moron to run Gaim with the suid bit set in the first place.

Modified Paths:
--------------
    trunk/src/protocols/silc/util.c
Modified: trunk/src/protocols/silc/util.c
===================================================================
--- trunk/src/protocols/silc/util.c	2006-04-18 21:16:45 UTC (rev 16060)
+++ trunk/src/protocols/silc/util.c	2006-04-19 02:12:45 UTC (rev 16061)
@@ -75,6 +75,7 @@
 	char pkd[256], prd[256];
 	struct stat st;
 	struct passwd *pw;
+	int fd;
 
 	pw = getpwuid(getuid());
 	if (!pw) {
@@ -225,6 +226,7 @@
 	}
 #endif
 
+	fd = open(file_private_key, O_RDONLY);
 	if ((g_stat(file_private_key, &st)) == -1) {
 		/* If file doesn't exist */
 		if (errno == ENOENT) {
@@ -234,10 +236,15 @@
 					     file_public_key, file_private_key, NULL,
 					     (gc->password == NULL) ? "" : gc->password,
 						 NULL, NULL, NULL, FALSE);
+			if (fd != -1)
+				close(fd);
+			fd = open(file_private_key, O_RDONLY);
 			g_stat(file_private_key, &st);
 		} else {
 			gaim_debug_error("silc", "Couldn't stat '%s' private key, error: %s\n",
 							 file_private_key, strerror(errno));
+			if (fd != -1)
+				close(fd);
 			return FALSE;
 		}
 	}
@@ -246,23 +253,30 @@
 	/* Check the owner of the private key */
 	if (st.st_uid != 0 && st.st_uid != pw->pw_uid) {
 		gaim_debug_error("silc", "You don't seem to own your private key!?\n");
+		if (fd != -1)
+			close(fd);
 		return FALSE;
 	}
 
 	/* Check the permissions for the private key */
 	if ((st.st_mode & 0777) != 0600) {
 		gaim_debug_warning("silc", "Wrong permissions in your private key file `%s'!\n"
-			"Trying to change them ... ", file_private_key);
-		if ((chmod(file_private_key, 0600)) == -1) {
+			"Trying to change them ...\n", file_private_key);
+		if ((fd != -1) && (fchmod(fd, S_IRUSR | S_IWUSR)) == -1) {
 			gaim_debug_error("silc",
 				"Failed to change permissions for private key file!\n"
 				"Permissions for your private key file must be 0600.\n");
+			if (fd != -1)
+				close(fd);
 			return FALSE;
 		}
 		gaim_debug_warning("silc", "Done.\n\n");
 	}
 #endif
 
+	if (fd != -1)
+		close(fd);
+
 	return TRUE;
 }
 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.