[565] in linux-security and linux-alert archive
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);