Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 8 Nov 2011 12:33:42 +0000
From: David Holland <dholland-oss-security@...bsd.org>
To: oss-security@...ts.openwall.com
Cc: sestoft@....dk
Subject: Re: caml-light insecure temporary files

On Sun, Nov 06, 2011 at 10:59:34PM +0100, Florian Weimer wrote:
 > > I don't know if anyone besides us still ships caml-light; it is long
 > > dead upstream and obsoleted by ocaml. AFAICT neither Debian nor Red
 > > Hat does. But just in case: it uses mktemp() insecurely, and also does
 > > unsafe things in /tmp during make install.
 > 
 > Moscow ML includes a copy of the affected code, and it's perhaps less
 > obsolete than caml-light.

Blah, it does indeed... here's the corresponding patch for it. Note
that mosml also requires a makefile patch similar to the one
referenced in http://gnats.netbsd.org/45558 to avoid scribbling in
/tmp when it installs.

Cc'd to the mosml maintainer.

--- mosmlyac/main.c.orig	2000-04-28 09:38:45.000000000 +0000
+++ mosmlyac/main.c
@@ -1,6 +1,9 @@
 #include <signal.h>
 #ifdef ANSI
 #include <string.h>
+#include <stdlib.h>
+#else
+extern char *getenv();
 #endif
 #include "defs.h"
 
@@ -33,6 +36,11 @@ char *text_file_name;
 char *union_file_name;
 char *verbose_file_name;
 
+static int action_fd = -1;
+static int entry_fd = -1;
+static int text_fd = -1;
+static int union_fd = -1;
+
 FILE *action_file;	/*  a temp file, used to save actions associated    */
 			/*  with rules until the parser is written	    */
 FILE *entry_file;
@@ -71,9 +79,6 @@ char  *rassoc;
 short **derives;
 char *nullable;
 
-extern char *mktemp();
-extern char *getenv();
-
 
 void done(int k)
 {
@@ -276,12 +281,21 @@ void create_file_names(void)
     union_file_name[len + 5] = 'u';
 
 #ifndef NO_UNIX
-    mktemp(action_file_name);
-    mktemp(entry_file_name);
-    mktemp(text_file_name);
-    mktemp(union_file_name);
+    action_fd = mkstemp(action_file_name);
+    entry_fd = mkstemp(entry_file_name);
+    text_fd = mkstemp(text_file_name);
+    union_fd = mkstemp(union_file_name);
 #endif
 
+    if (action_fd < 0)
+	open_error(action_file_name);
+    if (entry_fd < 0)
+	open_error(entry_file_name);
+    if (text_fd < 0)
+	open_error(text_file_name);
+    if (union_fd < 0)
+	open_error(union_file_name);
+
     len = strlen(file_prefix);
 
     output_file_name = MALLOC(len + 7);
@@ -321,15 +335,15 @@ void open_files(void)
 	    open_error(input_file_name);
     }
 
-    action_file = fopen(action_file_name, "w");
+    action_file = fdopen(action_fd, "w");
     if (action_file == 0)
 	open_error(action_file_name);
 
-    entry_file = fopen(entry_file_name, "w");
+    entry_file = fdopen(entry_fd, "w");
     if (entry_file == 0)
 	open_error(entry_file_name);
 
-    text_file = fopen(text_file_name, "w");
+    text_file = fdopen(text_fd, "w");
     if (text_file == 0)
 	open_error(text_file_name);
 
@@ -345,7 +359,7 @@ void open_files(void)
 	defines_file = fopen(defines_file_name, "w");
 	if (defines_file == 0)
 	    open_error(defines_file_name);
-	union_file = fopen(union_file_name, "w");
+	union_file = fdopen(union_fd, "w");
 	if (union_file ==  0)
 	    open_error(union_file_name);
     }


-- 
David A. Holland
dholland@...bsd.org

Powered by blists - more mailing lists

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.