changeset 13659:a92263b13380

[gaim-migrate @ 16061] 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. committer: Tailor Script <tailor@pidgin.im>
author Mark Doliner <mark@kingant.net>
date Wed, 19 Apr 2006 02:12:45 +0000
parents 788580688610
children f407d99481b8
files src/protocols/silc/util.c
diffstat 1 files changed, 16 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/src/protocols/silc/util.c	Tue Apr 18 21:16:45 2006 +0000
+++ b/src/protocols/silc/util.c	Wed Apr 19 02:12:45 2006 +0000
@@ -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;
 }