Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

insecure temp file handling in yacc #4797

Closed
vicuna opened this issue May 19, 2009 · 3 comments
Closed

insecure temp file handling in yacc #4797

vicuna opened this issue May 19, 2009 · 3 comments
Labels
Milestone

Comments

@vicuna
Copy link

vicuna commented May 19, 2009

Original bug ID: 4797
Reporter: @avsm
Assigned to: meyer
Status: closed (set by @xavierleroy on 2015-12-11T18:24:07Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 3.11.0
Target version: 4.01.0+dev
Category: ~DO NOT USE (was: OCaml general)
Monitored by: mehdi "Richard Jones"

Bug description

We have a patch in the OpenBSD OCaml port to use mkstemp(3) instead of mktemp(3), to avoid insecure temporary file creation. mkstemp(3) is fairly widespread these days. but our patch is guarded by a #define(OPENBSD) which probably needs to be replaced with something better.

http://www.openbsd.org/cgi-bin/cvsweb/ports/lang/ocaml/patches/patch-yacc_main_c?rev=1.5

--- yacc/main.c.orig Mon Jan 23 17:38:43 2006
+++ yacc/main.c Tue May 2 11:34:56 2006
@@ -55,6 +55,14 @@ char *text_file_name;
char *union_file_name;
char *verbose_file_name;

+#if defined(OpenBSD)
+#define HAVE_MKSTEMP
+#endif
+
+#ifdef HAVE_MKSTEMP
+int action_fd = -1, entry_fd = -1, text_fd = -1, union_fd = -1;
+#endif
+
FILE action_file; / a temp file, used to save actions associated /
/
with rules until the parser is written */
FILE *entry_file;
@@ -93,16 +101,29 @@ char *rassoc;
short **derives;
char *nullable;

+#if !defined(HAVE_MKSTEMP)
extern char *mktemp(char *);
+#endif
extern char *getenv(const char *);

void done(int k)
{
+#ifdef HAVE_MKSTEMP

  • if (action_fd != -1)
  •   unlink(action_file_name);
    
  • if (entry_fd != -1)
  •   unlink(entry_file_name);
    
  • if (text_fd != -1)
  •   unlink(text_file_name);
    
  • if (union_fd != -1)
  •   unlink(union_file_name);
    

+#else
if (action_file) { fclose(action_file); unlink(action_file_name); }
if (entry_file) { fclose(entry_file); unlink(entry_file_name); }
if (text_file) { fclose(text_file); unlink(text_file_name); }
if (union_file) { fclose(union_file); unlink(union_file_name); }
+#endif
if (output_file && k > 0) {
fclose(output_file); unlink(output_file_name);
}
@@ -302,11 +323,26 @@ void create_file_names(void)
union_file_name[len + 5] = 'u';

#ifndef NO_UNIX
+#ifdef HAVE_MKSTEMP

  • action_fd = mkstemp(action_file_name);
  • if (action_fd == -1)
  •    open_error(action_file_name);
    
  • entry_fd = mkstemp(entry_file_name);
  • if (entry_fd == -1)
  •    open_error(entry_file_name);
    
  • text_fd = mkstemp(text_file_name);
  • if (text_fd == -1)
  •    open_error(text_file_name);
    
  • union_fd = mkstemp(union_file_name);
  • if (union_fd == -1)
  •    open_error(union_file_name);
    

+#else
mktemp(action_file_name);
mktemp(entry_file_name);
mktemp(text_file_name);
mktemp(union_file_name);
#endif
+#endif

 len = strlen(file_prefix);

@@ -347,15 +383,27 @@ void open_files(void)
open_error(input_file_name);
}

+#ifdef HAVE_MKSTEMP

  • action_file = fdopen(action_fd, "w");
    +#else
    action_file = fopen(action_file_name, "w");
    +#endif
    if (action_file == 0)
    open_error(action_file_name);

+#ifdef HAVE_MKSTEMP

  • entry_file = fdopen(entry_fd, "w");
    +#else
    entry_file = fopen(entry_file_name, "w");
    +#endif
    if (entry_file == 0)
    open_error(entry_file_name);

+#ifdef HAVE_MKSTEMP

  • text_file = fdopen(text_fd, "w");
    +#else
    text_file = fopen(text_file_name, "w");
    +#endif
    if (text_file == 0)
    open_error(text_file_name);

@@ -371,7 +419,11 @@ void open_files(void)
defines_file = fopen(defines_file_name, "w");
if (defines_file == 0)
open_error(defines_file_name);
+#ifdef HAVE_MKSTEMP

  •    union_file = fdopen(union_fd, "w");
    

+#else
union_file = fopen(union_file_name, "w");
+#endif
if (union_file == 0)
open_error(union_file_name);
}

@vicuna
Copy link
Author

vicuna commented Sep 17, 2012

Comment author: @damiendoligez

Note that you shouldn't be running ocamlyacc (or any of the OCaml tools, really) in an insecure environment anyway.

@vicuna
Copy link
Author

vicuna commented Jun 17, 2013

Comment author: @gasche

This was fixed by Pierre Weis in SVN commit 9626.

@vicuna
Copy link
Author

vicuna commented Sep 13, 2013

Comment author: Richard Jones

Could this patch be modified slightly so it uses mkstemp on Linux as well:

https://git.fedorahosted.org/cgit/fedora-ocaml.git/commit/?id=d7be8e44a95e0c7fad0987869999ecbed8c20769

@vicuna vicuna closed this as completed Dec 11, 2015
@vicuna vicuna added this to the 4.01.0 milestone Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant