[565] in linux-security and linux-alert archive

home help back first fref pref prev next nref lref last post

fvwm fix

daemon@ATHENA.MIT.EDU (Zoltan Hidvegi)
Sun Jan 14 16:34:34 1996

From: Zoltan Hidvegi <hzoli@cs.elte.hu>
To: linux-security@tarsier.cv.nrao.edu, nation@rocket.sanders.lockheed.com
Date: Sun, 14 Jan 1996 01:49:45 +0100 (MET)

[Mod: Forwarded without comment or inspection of the code by me.  Please
direct all replies, comments, critiques, fixes, etc., to the author of
the post and not to the list.  Thanks.  --Jeff.]

Here is a patch to base fvwm-1.24r which hopefully fixes the security problem
discussed in the linux-security mailing list.  I am aware of the existing
debian fix but I do not like it.  This patch improves fvwm a little bit since
it avoids the popen call.  It uses pipe/fork/dup/exec.  This makes it easier
to pass options with spaces or shell special characters to m4.

As far as I know the O_CREAT | O_EXCL flags to open are enough to prevent the
attacks described earlier.  Please tell me if I'm wrong.  Send a Cc if you
reply to the list since I only read the digests.

Cheers,
Zoltan

*** configure.h	1996/01/13 11:48:44	1.1
--- configure.h	1995/12/31 02:21:26
***************
*** 1,7 ****
  #define FVWMDIR     "/usr/lib/X11/fvwm"
  /* #define FVWMDIR        "/local/homes/dsp/nation/modules"*/
  #define FVWM_ICONDIR   "/usr/include/X11/bitmaps:/usr/include/X11/pixmaps"
! #define FVWMRC         "/usr/lib/X11/fvwm/system.fvwmrc"
  
  /* Imake command needed to put modules in desired target location */
  /* Use the second version if it causes grief */
--- 1,7 ----
  #define FVWMDIR     "/usr/lib/X11/fvwm"
  /* #define FVWMDIR        "/local/homes/dsp/nation/modules"*/
  #define FVWM_ICONDIR   "/usr/include/X11/bitmaps:/usr/include/X11/pixmaps"
! #define FVWMRC         FVWMDIR "/system.fvwmrc"
  
  /* Imake command needed to put modules in desired target location */
  /* Use the second version if it causes grief */
***************
*** 69,75 ****
   *            undefine(`include') to fix that one. Some version of m4
   *            seem to give good error messages, others don't?
   ***************************************************************************/
! /* #define M4                          */
  
  /***************************************************************************
   *#define NO_PAGER 
--- 69,75 ----
   *            undefine(`include') to fix that one. Some version of m4
   *            seem to give good error messages, others don't?
   ***************************************************************************/
! #define M4
  
  /***************************************************************************
   *#define NO_PAGER 
***************
*** 113,119 ****
   *       
   *
   ***************************************************************************/
! /* #define PRUNE                       */
  
  /*************************************************************************
   *
--- 113,119 ----
   *       
   *
   ***************************************************************************/
! #define PRUNE
  
  /*************************************************************************
   *
*** fvwm/configure.c	1996/01/13 11:48:59	1.1
--- fvwm/configure.c	1996/01/13 14:06:02
***************
*** 300,306 ****
  char *fvwm_file;
  
  #ifdef M4
! static char *m4_defs(Display*, const char*, char*, char*);
  #endif
  
  /*****************************************************************************
--- 300,306 ----
  char *fvwm_file;
  
  #ifdef M4
! static char *m4_defs(Display*, const char*, char**, char*);
  #endif
  
  /*****************************************************************************
***************
*** 308,314 ****
   * This routine is responsible for reading and parsing the config file
   *
   ****************************************************************************/
! void MakeMenus(const char *display_name, char *m4_options)
  {
    char *system_file = FVWMRC;
    char *home_file;
--- 308,314 ----
   * This routine is responsible for reading and parsing the config file
   *
   ****************************************************************************/
! void MakeMenus(const char *display_name, char **m4_args)
  {
    char *system_file = FVWMRC;
    char *home_file;
***************
*** 390,396 ****
         * results in a temp file.
         */
    
!       fvwm_file = m4_defs(dpy, display_name, m4_options, fvwm_file);
        fclose(config_fd);
    
        config_fd = fopen(fvwm_file, "r"); 
--- 390,396 ----
         * results in a temp file.
         */
    
!       fvwm_file = m4_defs(dpy, display_name, m4_args, fvwm_file);
        fclose(config_fd);
    
        config_fd = fopen(fvwm_file, "r"); 
***************
*** 1891,1897 ****
  #define EXTRA   50
  
  extern int m4_prefix;
- extern char m4_prog[];
  extern int m4_default_quotes;
  extern char m4_startquote[];
  extern char m4_endquote[];
--- 1891,1896 ----
***************
*** 1964,1970 ****
      return(MkDef(name, num));
  }
  
! static char *m4_defs(Display *display, const char *host, char *m4_options, char *config_file)
  {
      Screen *screen;
      Visual *visual;
--- 1963,1969 ----
      return(MkDef(name, num));
  }
  
! static char *m4_defs(Display *display, const char *host, char **m4_args, char *config_file)
  {
      Screen *screen;
      Visual *visual;
***************
*** 1976,1981 ****
--- 1975,1983 ----
      char *vc;			/* Visual Class */
      FILE *tmpf;
      struct passwd *pwent;
+     int fd, pipefds[2], status;
+     pid_t child;
+ 
      /* Generate a temporary filename.  Honor the TMPDIR environment variable,
         if set. Hope nobody deletes this file! */
  
***************
*** 1984,2011 ****
      } else {
        strcpy(tmp_name, "/tmp");
      }
!     strcat(tmp_name, "/fvwmrcXXXXX");
!     mktemp(tmp_name);
!     
!     if (*tmp_name == '\0')
        {
  	perror("mktemp failed in m4_defs");
  	exit(0377);
        }
  
!     /*
!      * Create the appropriate command line to run m4, and
!      * open a pipe to the command.
!      */
  
!     sprintf(options, "%s %s %s > %s\n",
! 	    m4_prog,
! 	    ((m4_prefix == 0) ? "" : "--prefix-builtins"),
! 	    m4_options, tmp_name);
  
!     tmpf = popen(options, "w");
      if (tmpf == NULL) {
  	perror("Cannot open pipe to m4");
  	exit(0377);
      }
      
--- 1986,2032 ----
      } else {
        strcpy(tmp_name, "/tmp");
      }
!     strcat(tmp_name, "/fvwmrcXXXXXX");
! 
!     if (! mktemp(tmp_name) ||
! 	(fd = open(tmp_name, O_WRONLY | O_CREAT | O_EXCL, 0600)) == -1)
        {
  	perror("mktemp failed in m4_defs");
  	exit(0377);
        }
  
!     if (pipe(pipefds)) {
! 	perror("Cannot open pipe to m4");
! 	close(fd);
! 	unlink(tmp_name);
! 	exit(0377);
!     }
  
!     switch (child = fork()) {
!     case -1:
! 	close(fd);
! 	unlink(tmp_name);
! 	perror("fork failed in m4_defs");
! 	exit(0377);
!     case 0:
! 	close(STDIN_FILENO);
! 	dup(pipefds[0]);
! 	close(STDOUT_FILENO);
! 	dup(fd);
! 	close(pipefds[0]);
! 	close(pipefds[1]);
! 	close(fd);
! 	execvp(m4_args[0], m4_args);
! 	perror("failed to exec m4");
! 	exit(0377);
!     }
  
!     close(fd);
!     close(pipefds[0]);
!     tmpf = fdopen(pipefds[1], "w");
      if (tmpf == NULL) {
  	perror("Cannot open pipe to m4");
+ 	unlink(tmp_name);
  	exit(0377);
      }
      
***************
*** 2134,2140 ****
  	    config_file,
  	    m4_endquote);
      
!     pclose(tmpf);
      return(tmp_name);
  }
  #endif /* M4 */
--- 2155,2172 ----
  	    config_file,
  	    m4_endquote);
      
!     fclose(tmpf);
!     if (waitpid(child, &status, 0) != child) {
! 	perror("error while waiting for child");
! 	unlink(tmp_name);
! 	exit(0377);
!     }
!     if (! WIFEXITED(status) || WEXITSTATUS(status)) {
! 	fprintf(stderr, "%s exited in an unexpected way. status = %d\n",
! 		m4_args[0], status);
! 	unlink(tmp_name);
! 	exit(0377);
!     }
      return(tmp_name);
  }
  #endif /* M4 */
*** fvwm/functions.c	1996/01/13 12:14:39	1.1
--- fvwm/functions.c	1996/01/13 13:25:32
***************
*** 1643,1649 ****
  
  void QuickRestart(void)
  {
!   extern char *m4_options;
    extern char *display_name;
    int i;
    FvwmWindow *tmp,*next;  
--- 1643,1649 ----
  
  void QuickRestart(void)
  {
!   extern char **m4_args;
    extern char *display_name;
    int i;
    FvwmWindow *tmp,*next;  
***************
*** 1684,1690 ****
    InitVariables();
  
  #ifdef M4
!   MakeMenus(display_name, m4_options);
  #else
    MakeMenus(display_name, NULL);
  #endif  
--- 1684,1690 ----
    InitVariables();
  
  #ifdef M4
!   MakeMenus(display_name, m4_args);
  #else
    MakeMenus(display_name, NULL);
  #endif  
*** fvwm/fvwm.c	1996/01/13 12:14:39	1.1
--- fvwm/fvwm.c	1996/01/13 13:46:18
***************
*** 102,111 ****
  char **g_argv;
  
  #ifdef M4
  int  m4_enable;			/* use m4? */
  int  m4_prefix;			/* Do GNU m4 prefixing (-P) */
! char m4_options[BUFSIZ];	/* Command line options to m4 */
! char m4_prog[BUFSIZ];		/* Name of the m4 program */
  int  m4_default_quotes;		/* Use default m4 quotes */
  char m4_startquote[16];		/* Left quote characters for m4 */
  char m4_endquote[16];		/* Right quote characters for m4 */
--- 102,112 ----
  char **g_argv;
  
  #ifdef M4
+ #define MAX_ARGS (BUFSIZ / sizeof(char *))
  int  m4_enable;			/* use m4? */
  int  m4_prefix;			/* Do GNU m4 prefixing (-P) */
! char *m4_args[MAX_ARGS] = { "m4", NULL }; /* Command line options to m4 */
! int  m4_argcount = 1;		/* Current number of args in m4_args */
  int  m4_default_quotes;		/* Use default m4 quotes */
  char m4_startquote[16];		/* Left quote characters for m4 */
  char m4_endquote[16];		/* Right quote characters for m4 */
***************
*** 154,161 ****
       
      m4_enable = TRUE;
      m4_prefix = FALSE;
-     strcpy(m4_prog, "m4");
-     *m4_options = '\0';
      m4_default_quotes = 1;
      strcpy(m4_startquote, "`");
      strcpy(m4_endquote, "'");
--- 155,160 ----
***************
*** 187,201 ****
  	    m4_enable = FALSE;
  	  }
  	else if (mystrncasecmp(argv[i], "-m4-prefix", 10) == 0)
! 	  {	
  	    m4_prefix = TRUE;
  	  }
  	else if (mystrncasecmp(argv[i],"-m4opt", 6) == 0)
  	  {
! 	    if (++i < argc) 
  	      {
! 		strcat(m4_options, argv[i]);
! 		strcat(m4_options, " ");	    
  	      }
  	  }
  	else if (mystrncasecmp(argv[i], "-m4-squote", 6) == 0)
--- 186,205 ----
  	    m4_enable = FALSE;
  	  }
  	else if (mystrncasecmp(argv[i], "-m4-prefix", 10) == 0)
! 	  {
! 	    if (! m4_prefix && m4_argcount < MAX_ARGS - 1)
! 	      {
! 		m4_args[m4_argcount++] = "--prefix-builtins";
! 		m4_args[m4_argcount] = NULL;
! 	      }
  	    m4_prefix = TRUE;
  	  }
  	else if (mystrncasecmp(argv[i],"-m4opt", 6) == 0)
  	  {
! 	    if (++i < argc && m4_argcount < MAX_ARGS - 1)
  	      {
! 		m4_args[m4_argcount++] = argv[i];
! 		m4_args[m4_argcount] = NULL;
  	      }
  	  }
  	else if (mystrncasecmp(argv[i], "-m4-squote", 6) == 0)
***************
*** 218,224 ****
  	  {
  	    if (++i < argc)
  	      {
! 		strcpy(m4_prog, argv[i]);
  	      }
  	  }
  #endif
--- 222,228 ----
  	  {
  	    if (++i < argc)
  	      {
! 		m4_args[0] = argv[i];
  	      }
  	  }
  #endif
***************
*** 362,368 ****
  
      /* read config file, set up menus, colors, fonts */
  #ifdef M4
!     MakeMenus(display_name, m4_options);
  #else
      MakeMenus(display_name, NULL);
  #endif
--- 366,372 ----
  
      /* read config file, set up menus, colors, fonts */
  #ifdef M4
!     MakeMenus(display_name, m4_args);
  #else
      MakeMenus(display_name, NULL);
  #endif
*** fvwm/fvwm.man	1996/01/13 14:49:13	1.1
--- fvwm/fvwm.man	1996/01/13 14:51:05
***************
*** 410,419 ****
  If GNU \fIm4\fP is available, cause \fIm4\fP to prefix all builtin commands with
  \fIm4_\fP.
  .IP "\fB-m4opt\fP \fIoption\fP"
! Pass this option to \fIm4\fP.  The \fIoption\fP can be any string of
! characters without spaces.  This option can occur multiple times.  If
! GNU \fIm4\fP is available, \fBDO NOT\fP pass the \fI-P\fP option here.
! Use \fB-m4-prefix\fP instead.
  .IP "\fB-m4-squote\fP \fIstring\fP"
  Use this given \fBstring\fP as the starting quote characters.  You
  must also specify \fB-m4-equote\fI.
--- 410,418 ----
  If GNU \fIm4\fP is available, cause \fIm4\fP to prefix all builtin commands with
  \fIm4_\fP.
  .IP "\fB-m4opt\fP \fIoption\fP"
! Pass this option to \fIm4\fP.  This option can occur multiple times.
! If GNU \fIm4\fP is available, \fBDO NOT\fP pass the \fI-P\fP option
! here.  Use \fB-m4-prefix\fP instead.
  .IP "\fB-m4-squote\fP \fIstring\fP"
  Use this given \fBstring\fP as the starting quote characters.  You
  must also specify \fB-m4-equote\fI.
*** fvwm/misc.h	1996/01/13 13:24:31	1.1
--- fvwm/misc.h	1996/01/13 13:25:22
***************
*** 130,136 ****
  extern void       FetchWmColormapWindows (FvwmWindow *tmp);
  extern void       PaintEntry(MenuRoot *, MenuItem *);
  extern void       PaintMenu(MenuRoot *, XEvent *);
! extern void       MakeMenus(const char*, char*);
  extern void       InitEvents(void);
  extern void       DispatchEvent(void);
  extern void       HandleEvents(void);
--- 130,136 ----
  extern void       FetchWmColormapWindows (FvwmWindow *tmp);
  extern void       PaintEntry(MenuRoot *, MenuItem *);
  extern void       PaintMenu(MenuRoot *, XEvent *);
! extern void       MakeMenus(const char*, char**);
  extern void       InitEvents(void);
  extern void       DispatchEvent(void);
  extern void       HandleEvents(void);

home help back first fref pref prev next nref lref last post