xmonisdn was meant to be installed setuid root. However, this was not secure

(scripts were executed with system() so e.g. IFS and PATH could be used to
work around any attempt of security...).
Now check that it is not installed setuid root (unless the user insists,
details in manpage), and recommend to put user in group `dialout' or similar
and also make the isdn devices group `dialout'; the scripts can then use
isdnctrl to stop/start stuff.
This commit is contained in:
paul 1999-08-03 14:00:18 +00:00
parent c841376b7d
commit 7ede89aeab
6 changed files with 114 additions and 26 deletions

View File

@ -7,7 +7,7 @@ LOCAL_LIBRARIES = XawClientLibs
OBJS = xmonisdn.o Net.o
HEADERS = NetP.h Net.h
INSTPGMFLAGS = -m 4755 -s
INSTPGMFLAGS = -m 0755 -s
ComplexProgramTarget(xmonisdn)

View File

@ -1,14 +1,25 @@
#
# This Makefile is used for compatibility to the
# isdn4k-utils package. In order to superseede
# the original Makefile, it is called makefile.
# isdn4k-utils package. In order to supercede
# the original Makefile, it is called GNUmakefile.
#
#
# Add security check:
# - executed commands must be owned by root
# and only writable by owner.
# Additional security stuff:
#
# - don't be setuid root, and check for this; Bad Things can be done with
# setting IFS and/or PATH before invoking xmonisdn!
#
# - It's better to make the ISDN devices group `dialout' (or so), and put
# the user also into that group. Then no setuid/setgid bits are needed.
#
# - Cater for users who think they know what they are doing: refuse to run
# setuid root unless an option `-r' is given; hide this option in the
# docs where the problems with setuid programs are explained :-)
I4LU_DEFINES := $(CFLAGS) -DPARANOIA_CHECK -I.
ifeq (../.config,$(wildcard ../.config))
include ../.config

View File

@ -4,6 +4,7 @@
#include <X11/Xosdefs.h> /* for X_NOT_POSIX def */
#include <pwd.h> /* for getting username */
#include <sys/stat.h> /* for stat() ** needs types.h ***/
#include <unistd.h>
#include <stdio.h> /* for printing error messages */
#include <stdlib.h>
#include <sys/types.h>
@ -48,6 +49,8 @@ typedef union wait waitType;
#include <X11/Xmu/Drawing.h>
#include <X11/extensions/shape.h>
/* Only allow calling scripts when setuid root if the -r option is given */
extern int allow_setuid;
/*
* The default user interface is to have the netstat turn itself off whenever
* the user presses a button in it. Expert users might want to make this
@ -245,15 +248,23 @@ static int paranoia_check(cmd)
#ifdef PARANOIA_CHECK
struct stat stbuf;
if (stat(cmd, &stbuf))
/* only check if setuid root */
if (geteuid() != 0 || getuid() == 0)
return 1;
/* don't allow any scripts if setuid root and not called with -r option */
if (!allow_setuid)
return 0;
if (stat(cmd, &stbuf))
/* stat failed, stay on the safe side */
return 0;
return 0;
if (stbuf.st_uid != 0)
/* owner is not root */
return 0;
return 0;
if (stbuf.st_mode & (S_IWGRP | S_IWOTH))
/* writable by group or world */
return 0;
return 0;
#endif
return 1;
}
@ -271,10 +282,11 @@ static void NetUp (gw, event, params, nparams)
if ((w->netstat.state <= 1) && (!updownwait)) {
w->netstat.state = 5;
updownwait = 150 / w->netstat.update;
if (updownwait < 2) updownwait = 2;
if (updownwait < 2)
updownwait = 2;
redraw_netstat(w);
if (paranoia_check(NETUP_COMMAND))
system(NETUP_COMMAND);
system(NETUP_COMMAND);
}
return;
}
@ -294,9 +306,10 @@ static void NetDown (gw, event, params, nparams)
w->netstat.state = 6;
redraw_netstat(w);
updownwait = 150 / w->netstat.update;
if (updownwait < 2) updownwait = 2;
if (updownwait < 2)
updownwait = 2;
if (paranoia_check(NETDOWN_COMMAND))
system(NETDOWN_COMMAND);
system(NETDOWN_COMMAND);
}
return;
}
@ -592,9 +605,13 @@ static int parse_info()
infoptr = infoline + 7;
for (i=0; i<ISDN_MAX_CHANNELS; i++) {
sscanf(infoptr,"%d",&num);
if ((num&ISDN_USAGE_MASK) == ISDN_USAGE_NET)
if (num&ISDN_USAGE_OUTGOING) newstate = 4;
else if (newstate < 3) newstate = 3;
if ((num&ISDN_USAGE_MASK) == ISDN_USAGE_NET) {
if (num&ISDN_USAGE_OUTGOING)
newstate = 4;
else
if (newstate < 3)
newstate = 3;
}
sscanf(infoptr,"%s",temp);
infoptr = infoptr + strlen(temp) + 1;
}

View File

@ -40,8 +40,15 @@ Pressing button 2 in the window while the ISDN system is inactive
executes the command "/sbin/isdnnet start &" and pressing button 3
executes "/sbin/isdnnet stop". Provided /sbin/isdnnet is a
shell-script that can start and stop the ISDN subsystem and provided
xmonisdn is installed as setuid root, you can startup and stop the
ISDN system in this way.
you are in the dialout group (and the ISDN devices in /dev are also
in the dialout group) you can startup and stop the ISDN system in
this way.
An alternative is to install xmonisdn as setuid root, but that may
have serious security problems, so that is strongly discouraged.
If you insist on using xmonisdn as setuid root, read the manual
page on info how to do that. You could also install xmonisdn as
setgid dialout, but that too is not without possible problems.
4. Compilation and Installation
@ -52,8 +59,7 @@ After having installed the source directory type
make
make install.all
which compiles and installs all files. Note: xmonisdn is installed
as setiud root!
which compiles and installs all files.
Now you only have to setup the shell-script /sbin/isdnnet, which could
simply be a link to the appropriate initialization script. I wrote a
@ -62,7 +68,4 @@ all mail that has been queued while the ISDN system was inactive.
Bernhard Nebel, December 9, 1996
Modified August 3, 1999 by Paul Slootman <paul@isdn4linux.de>

View File

@ -1,3 +1,4 @@
#include <unistd.h>
#include <stdio.h>
#include <X11/Xatom.h>
#include <X11/Intrinsic.h>
@ -8,6 +9,9 @@
extern void exit();
static void quit();
/* Only allow calling scripts when setuid root if the -r option is given */
int allow_setuid = 0;
char *ProgramName;
static XrmOptionDescRec options[] = {
@ -56,6 +60,24 @@ int main (argc, argv)
ProgramName = argv[0];
/* check for first arg `-r'. Must be first! */
if (argc > 1 && !strcmp(argv[1], "-r")) {
int i;
allow_setuid = 1;
argc--;
for (i = 1; i < argc; i++)
argv[i] = argv[i+1];
argv[i] = 0;
}
else {
if (geteuid() == 0 && getuid() != 0) {
fprintf(stderr,
"`%s' setuid root not allowed! Read the manpage to find out why.\n",
ProgramName);
exit(1);
}
}
toplevel = XtAppInitialize(&xtcontext, "xmonisdn", options, XtNumber (options),
&argc, argv, NULL, NULL, 0);
if (argc != 1) Usage ();

View File

@ -1,6 +1,6 @@
.\" $Id: xmonisdn.man.in,v 1.2 1998/12/02 11:58:51 paul Exp $
.\" $Id: xmonisdn.man.in,v 1.3 1999/08/03 14:00:22 paul Exp $
.\"
.\" CHECKIN $Date: 1998/12/02 11:58:51 $
.\" CHECKIN $Date: 1999/08/03 14:00:22 $
.\"
.TH XMONISDN 1 "@MANDATE@" isdn4k-utils-@I4LVERSION@ "X Tools"
.SH NAME
@ -199,6 +199,41 @@ The default translation is
<ButtonPress>Button3: netdown()
.fi
.sp
.SH SECURITY ISSUES
There are a number of reasons why you should not run
.I xmonisdn
as a `setuid root' program.
Most importantly, any setuid root program is a potential security hole.
The scripts that are executed by
.I xmonisdn
may be compromised, or contain non-absolute paths to the commands called
(so that by changing the PATH variable the wrong binary is executed!);
or the script may in turn call other things that are not secure.
.br
If you are sure that your scripts are secure and you insist on making
.I xmonisdn
setuid root, you can give the
.B -r
option as
.I first
option. This will turn off xmonisdn's setuid detection. However, you are on
your own if you do this!
.br
A better alternative is to put the isdn devices in group
.B dialout
(if not so already), and add yourself to the
.B dialout
group. This way you have permission to manipulate the ISDN devices, e.g.
to use
.I isdnctrl
to change the dialmode setting to off (to disable dialing) or auto (for
autodial). The whole ISDN system may also be turned on and off, see the
.I isdnctrl manpage for more info.
This is a more secure way of controlling the dialing than e.g. by
manipulating routes.
.SH ENVIRONMENT
.PP
.TP 8
@ -211,4 +246,4 @@ stored in the RESOURCE_MANAGER property.
.SH AUTHOR
Bernhard Nebel
.SH SEE ALSO
.BR xmonisdn(1x)
.BR isdnctrl(8)