stat before open is flawed

3 files changed, 48 insertions, 15 deletions

diff --git a/sbin/isakmpd/policy.c b/sbin/isakmpd/policy.c
index 27f3ca32d1a..e2fb68512a7 100644
--- a/sbin/isakmpd/policy.c
+++ b/sbin/isakmpd/policy.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: policy.c,v 1.71 2004/04/28 20:20:31 hshoexer Exp $	 */
+/* $OpenBSD: policy.c,v 1.72 2004/05/23 16:14:22 deraadt Exp $	 */
 /* $EOM: policy.c,v 1.49 2000/10/24 13:33:39 niklas Exp $ */
 
 /*
@@ -1767,15 +1767,17 @@ policy_init(void)
 	if (!policy_file)
 		policy_file = CONF_DFLT_POLICY_FILE;
 
-	/* Check file modes and collect file size */
-	if (check_file_secrecy(policy_file, &sz))
-		log_fatal("policy_init: cannot read %s", policy_file);
-
 	/* Open policy file.  */
 	fd = monitor_open(policy_file, O_RDONLY, 0);
 	if (fd == -1)
 		log_fatal("policy_init: open (\"%s\", O_RDONLY) failed", policy_file);
 
+	/* Check file modes and collect file size */
+	if (check_file_secrecy_fd(fd, policy_file, &sz)) {
+		close(fd);
+		log_fatal("policy_init: cannot read %s", policy_file);
+	}
+
 	/* Allocate memory to keep policies.  */
 	ptr = calloc(sz + 1, sizeof(char));
 	if (!ptr)
@@ -2000,10 +2002,19 @@ keynote_cert_obtain(u_int8_t * id, size_t id_len, void *data, u_int8_t ** cert,
 		return 0;
 	}
 
-	if (monitor_stat(file, &sb) < 0) {
+	fd = monitor_open(file, O_RDONLY, 0);
+	if (fd < 0) {
+		LOG_DBG((LOG_POLICY, 30, "keynote_cert_obtain: failed to open \"%s\"",
+		    file));
+		free(file);
+		return 0;
+	}
+
+	if (fstat(fd, &sb) < 0) {
 		LOG_DBG((LOG_POLICY, 30, "keynote_cert_obtain: failed to stat \"%s\"",
 		    file));
 		free(file);
+		close(fd);
 		return 0;
 	}
 	size = (size_t)sb.st_size;
@@ -2015,13 +2026,7 @@ keynote_cert_obtain(u_int8_t * id, size_t id_len, void *data, u_int8_t ** cert,
 		free(file);
 		return 0;
 	}
-	fd = monitor_open(file, O_RDONLY, 0);
-	if (fd < 0) {
-		LOG_DBG((LOG_POLICY, 30, "keynote_cert_obtain: failed to open \"%s\"",
-		    file));
-		free(file);
-		return 0;
-	}
+
 	if (read(fd, *cert, size) != (int)size) {
 		LOG_DBG((LOG_POLICY, 30, "keynote_cert_obtain: failed to read %lu "
 		    "bytes from \"%s\"", (unsigned long)size, file));
diff --git a/sbin/isakmpd/util.c b/sbin/isakmpd/util.c
index c343bd0fec5..3209e116aef 100644
--- a/sbin/isakmpd/util.c
+++ b/sbin/isakmpd/util.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: util.c,v 1.37 2004/04/15 18:39:26 deraadt Exp $	 */
+/* $OpenBSD: util.c,v 1.38 2004/05/23 16:14:22 deraadt Exp $	 */
 /* $EOM: util.c,v 1.23 2000/11/23 12:22:08 niklas Exp $	 */
 
 /*
@@ -509,3 +509,30 @@ check_file_secrecy(char *name, size_t *file_size)
 
 	return 0;
 }
+
+int
+check_file_secrecy_fd(int fd, char *name, size_t *file_size)
+{
+	struct stat st;
+
+	if (fstat(fd, &st) == -1) {
+		log_error("check_file_secrecy: stat (\"%s\") failed", name);
+		return -1;
+	}
+	if (st.st_uid != 0 && st.st_uid != getuid()) {
+		log_print("check_file_secrecy: "
+		    "not loading %s - file owner is not process user", name);
+		errno = EPERM;
+		return -1;
+	}
+	if ((st.st_mode & (S_IRWXG | S_IRWXO)) != 0) {
+		log_print("conf_file_secrecy: not loading %s - too open permissions",
+		    name);
+		errno = EPERM;
+		return -1;
+	}
+	if (file_size)
+		*file_size = (size_t) st.st_size;
+
+	return 0;
+}
diff --git a/sbin/isakmpd/util.h b/sbin/isakmpd/util.h
index 5c04880e2c5..4d94788ecd8 100644
--- a/sbin/isakmpd/util.h
+++ b/sbin/isakmpd/util.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: util.h,v 1.18 2004/04/15 18:39:26 deraadt Exp $	 */
+/* $OpenBSD: util.h,v 1.19 2004/05/23 16:14:22 deraadt Exp $	 */
 /* $EOM: util.h,v 1.10 2000/10/24 13:33:39 niklas Exp $	 */
 
 /*
@@ -43,6 +43,7 @@ struct message;
 struct sockaddr;
 
 extern int      check_file_secrecy(char *, size_t *);
+extern int      check_file_secrecy_fd(int, char *, size_t *);
 extern u_int16_t decode_16(u_int8_t *);
 extern u_int32_t decode_32(u_int8_t *);
 extern u_int64_t decode_64(u_int8_t *);