Re: caml-light insecure temporary files

[David Holland <dholland-oss-security () netbsd org>]

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 () netbsd org