Date: Sat, 15 Sep 2001 23:31:25 -0400 (EDT) From: "Andrew R. Reiter" <arr@watson.org> To: freebsd-audit@freebsd.org Subject: sliplogin patches Message-ID: <Pine.NEB.3.96L.1010915232244.41506A-300000@fledge.watson.org>
index | next in thread | raw e-mail
[-- Attachment #1 --]
Hey,
I was looking at sliplogin() and noticed that the signal handler usage was
kinda poor (bad functions in hup_handler()). So I wanted to make patches
for it. After getting style 9'd (kinda like slashdotted, but you get
beaten up for not fully style(9)'ing something prepatch), i decided to
write the style(9) patch for good measure; hopefully it's good :-). So,
sliplogin.c.style-diff is the style(9) patch.
the security patch is sliplogin.c.sec-diff. this patch fixes the signal
race problem in the essence of the kkenn fix in syslogd (mark the signal,
check if it's been marked and execute -- to put it very pseudo-like).
also, I fixed probably some unnecesary things but thought it should be
done for "good measure" -- those are:
- moving strncpy() + buf[sizeof-1] = '\0' -> strlcpy()
- remove sizeof(blah) - 1 in fgets() where there _shouldn't_ be a
-1 (man 3 fgets).
anyway, like usual, comments are appreciated and probably needed (slowly
becomming style(9)-ful).
cheers,
Andrew
btw -- patches can also be seen at:
http://www.watson.org/~arr/fbsd-audit/usr.sbin/sliplogin/
*-------------.................................................
| Andrew R. Reiter
| arr@fledge.watson.org
| "It requires a very unusual mind
| to undertake the analysis of the obvious" -- A.N. Whitehead
[-- Attachment #2 --]
--- sliplogin.c.orig Sat Sep 15 20:46:44 2001
+++ sliplogin.c.1 Sat Sep 15 21:21:22 2001
@@ -97,6 +97,17 @@
NULL
};
+struct slip_modes {
+ char *sm_name;
+ int sm_or_flag;
+ int sm_and_flag;
+} modes[] = {
+ {"normal", 0, 0},
+ {"compress", IFF_LINK0, IFF_LINK2},
+ {"noicmp", IFF_LINK1, 0},
+ {"autocomp", IFF_LINK2, IFF_LINK0}
+};
+
int unit;
int slip_mode;
speed_t speed;
@@ -107,8 +118,8 @@
char loginargs[BUFSIZ];
char loginfile[MAXPATHLEN];
char loginname[BUFSIZ];
-static char raddr[32]; /* remote address */
-char ifname[IFNAMSIZ]; /* interface name */
+char ifname[IFNAMSIZ]; /* interface name */
+static char raddr[32]; /* remote address */
static char pidfilename[MAXPATHLEN]; /* name of pid file */
static char iffilename[MAXPATHLEN]; /* name of if file */
static pid_t pid; /* our pid */
@@ -116,34 +127,22 @@
char *
make_ipaddr(void)
{
-static char address[20] ="";
-struct hostent *he;
-unsigned long ipaddr;
-
-address[0] = '\0';
-if ((he = gethostbyname(raddr)) != NULL) {
- ipaddr = ntohl(*(long *)he->h_addr_list[0]);
- sprintf(address, "%lu.%lu.%lu.%lu",
- ipaddr >> 24,
- (ipaddr & 0x00ff0000) >> 16,
- (ipaddr & 0x0000ff00) >> 8,
- (ipaddr & 0x000000ff));
+ static char address[20] ="";
+ struct hostent *he;
+ unsigned long ipaddr;
+
+ address[0] = '\0';
+ if ((he = gethostbyname(raddr)) != NULL) {
+ ipaddr = ntohl(*(long *)he->h_addr_list[0]);
+ sprintf(address, "%lu.%lu.%lu.%lu",
+ ipaddr >> 24,
+ (ipaddr & 0x00ff0000) >> 16,
+ (ipaddr & 0x0000ff00) >> 8,
+ (ipaddr & 0x000000ff));
}
-
-return address;
+ return address;
}
-struct slip_modes {
- char *sm_name;
- int sm_or_flag;
- int sm_and_flag;
-} modes[] = {
- "normal", 0 , 0 ,
- "compress", IFF_LINK0, IFF_LINK2,
- "noicmp", IFF_LINK1, 0 ,
- "autocomp", IFF_LINK2, IFF_LINK0,
-};
-
void
findid(name)
char *name;
@@ -172,9 +171,11 @@
goto accfile_err;
if (loginargs[0] == '#' || isspace(loginargs[0]))
continue;
- n = sscanf(loginargs, "%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s\n",
- user, laddr, raddr, mask, slopt[0], slopt[1],
- slopt[2], slopt[3], slopt[4]);
+ n = sscanf(loginargs,
+ "%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]"
+ "%15s%*[ \t]%15s%*[ \t]%15s%*[ \t]%15s\n",
+ user, laddr, raddr, mask, slopt[0], slopt[1],
+ slopt[2], slopt[3], slopt[4]);
if (n < 4) {
syslog(LOG_ERR, "%s: wrong format\n", _PATH_ACCESS);
exit(1);
@@ -182,12 +183,12 @@
if (strcmp(user, name) != 0)
continue;
- (void) fclose(fp);
+ (void)fclose(fp);
slip_mode = 0;
for (i = 0; i < n - 4; i++) {
- for (j = 0; j < sizeof(modes)/sizeof(struct slip_modes);
- j++) {
+ for (j = 0;
+ j < sizeof(modes)/sizeof(struct slip_modes); j++) {
if (strcmp(modes[j].sm_name, slopt[i]) == 0) {
slip_mode |= (modes[j].sm_or_flag);
slip_mode &= ~(modes[j].sm_and_flag);
@@ -202,9 +203,11 @@
* one specific to this host. If none found, try for
* a generic one.
*/
- (void)snprintf(loginfile, sizeof(loginfile), "%s.%s", _PATH_LOGIN, name);
+ (void)snprintf(loginfile, sizeof(loginfile),
+ "%s.%s", _PATH_LOGIN, name);
if (access(loginfile, R_OK|X_OK) != 0) {
- (void)strncpy(loginfile, _PATH_LOGIN, sizeof(loginfile)-1);
+ (void)strncpy(loginfile, _PATH_LOGIN,
+ sizeof(loginfile)-1);
loginfile[sizeof(loginfile)-1] = '\0';
if (access(loginfile, R_OK|X_OK)) {
syslog(LOG_ERR,
@@ -213,9 +216,11 @@
exit(5);
}
}
- (void)snprintf(slparmsfile, sizeof(slparmsfile), "%s.%s", _PATH_SLPARMS, name);
+ (void)snprintf(slparmsfile, sizeof(slparmsfile),
+ "%s.%s", _PATH_SLPARMS, name);
if (access(slparmsfile, R_OK|X_OK) != 0) {
- (void)strncpy(slparmsfile, _PATH_SLPARMS, sizeof(slparmsfile)-1);
+ (void)strncpy(slparmsfile, _PATH_SLPARMS,
+ sizeof(slparmsfile)-1);
slparmsfile[sizeof(slparmsfile)-1] = '\0';
if (access(slparmsfile, R_OK|X_OK))
*slparmsfile = '\0';
@@ -234,19 +239,20 @@
goto slfile_err;
if (buf[0] == '#' || isspace(buf[0]))
continue;
- n = sscanf(buf, "%d %d %d", &keepal, &outfill, &slunit);
+ n = sscanf(buf, "%d %d %d",
+ &keepal, &outfill, &slunit);
if (n < 1) {
slwrong_fmt:
- syslog(LOG_ERR, "%s: wrong format\n", slparmsfile);
+ syslog(LOG_ERR, "%s: wrong format\n",
+ slparmsfile);
exit(1);
}
- (void) fclose(fp);
+ (void)fclose(fp);
break;
}
if (n == 0)
goto slwrong_fmt;
}
-
return;
}
syslog(LOG_ERR, "SLIP access denied for %s\n", name);
@@ -304,22 +310,23 @@
int s;
{
char logoutfile[MAXPATHLEN];
+ char logincmd[2*MAXPATHLEN+32];
- (void) close(0);
+ (void)close(0);
seteuid(0);
- (void)snprintf(logoutfile, sizeof(logoutfile), "%s.%s", _PATH_LOGOUT, loginname);
+ (void)snprintf(logoutfile, sizeof(logoutfile),
+ "%s.%s", _PATH_LOGOUT, loginname);
if (access(logoutfile, R_OK|X_OK) != 0) {
(void)strncpy(logoutfile, _PATH_LOGOUT, sizeof(logoutfile)-1);
logoutfile[sizeof(logoutfile)-1] = '\0';
}
if (access(logoutfile, R_OK|X_OK) == 0) {
- char logincmd[2*MAXPATHLEN+32];
-
- (void) snprintf(logincmd, sizeof(logincmd), "%s %d %ld %s", logoutfile, unit, speed, loginargs);
- (void) system(logincmd);
+ (void)snprintf(logincmd, sizeof(logincmd),
+ "%s %d %u %s", logoutfile, unit, speed, loginargs);
+ (void)system(logincmd);
}
- syslog(LOG_INFO, "closed %s slip unit %d (%s)\n", loginname, unit,
- sigstr(s));
+ syslog(LOG_INFO, "closed %s slip unit %d (%s)\n",
+ loginname, unit, sigstr(s));
if (unlink(pidfilename) < 0 && errno != ENOENT)
syslog(LOG_WARNING, "unable to delete pid file: %m");
if (unlink(iffilename) < 0 && errno != ENOENT)
@@ -328,9 +335,9 @@
/* NOTREACHED */
}
-
/* Modify the slip line mode and add any compression or no-icmp flags. */
-void line_flags(unit)
+void
+line_flags(unit)
int unit;
{
struct ifreq ifr;
@@ -361,7 +368,7 @@
close(s);
}
-
+int
main(argc, argv)
int argc;
char *argv[];
@@ -382,7 +389,7 @@
name = argv[0];
s = getdtablesize();
for (fd = 3 ; fd < s ; fd++)
- (void) close(fd);
+ (void)close(fd);
openlog(name, LOG_PID|LOG_PERROR, LOG_DAEMON);
uid = getuid();
if (argc > 1) {
@@ -401,7 +408,7 @@
syslog(LOG_ERR, "open %s: %m", argv[2]);
exit(2);
}
- (void) dup2(fd, 0);
+ (void)dup2(fd, 0);
if (fd > 2)
close(fd);
}
@@ -420,11 +427,11 @@
}
findid(name);
}
- (void) fchmod(0, 0600);
- (void) fprintf(stderr, "starting slip login for %s\n", loginname);
- (void) fprintf(stderr, "your address is %s\n\n", make_ipaddr());
+ (void)fchmod(0, 0600);
+ (void)fprintf(stderr, "starting slip login for %s\n", loginname);
+ (void)fprintf(stderr, "your address is %s\n\n", make_ipaddr());
- (void) fflush(stderr);
+ (void)fflush(stderr);
sleep(1);
/* set up the line parameters */
@@ -454,8 +461,8 @@
syslog(LOG_ERR, "ioctl (SLIOCGUNIT): %m");
exit(1);
}
- (void) signal(SIGHUP, hup_handler);
- (void) signal(SIGTERM, hup_handler);
+ (void)signal(SIGHUP, hup_handler);
+ (void)signal(SIGTERM, hup_handler);
if (keepal > 0) {
(void) signal(SIGURG, hup_handler);
@@ -471,14 +478,14 @@
/* write pid to file */
pid = getpid();
- (void) sprintf(ifname, "sl%d", unit);
- (void) sprintf(pidfilename, "%s%s.pid", _PATH_VARRUN, ifname);
+ (void)sprintf(ifname, "sl%d", unit);
+ (void)sprintf(pidfilename, "%s%s.pid", _PATH_VARRUN, ifname);
if ((pidfile = fopen(pidfilename, "w")) != NULL) {
fprintf(pidfile, "%d\n", pid);
- (void) fclose(pidfile);
+ (void)fclose(pidfile);
} else {
syslog(LOG_ERR, "Failed to create pid file %s: %m",
- pidfilename);
+ pidfilename);
pidfilename[0] = 0;
}
@@ -491,10 +498,10 @@
n++;
break;
}
- (void) sprintf(iffilename, "%s%s.if", _PATH_VARRUN, &devnam[n]);
+ (void)sprintf(iffilename, "%s%s.if", _PATH_VARRUN, &devnam[n]);
if ((iffile = fopen(iffilename, "w")) != NULL) {
fprintf(iffile, "sl%d\n", unit);
- (void) fclose(iffile);
+ (void)fclose(iffile);
} else {
syslog(LOG_ERR, "Failed to create if file %s: %m", iffilename);
iffilename[0] = 0;
@@ -502,32 +509,33 @@
syslog(LOG_INFO, "attaching slip unit %d for %s\n", unit, loginname);
- (void)snprintf(logincmd, sizeof(logincmd), "%s %d %ld %s", loginfile, unit, speed,
- loginargs);
+ (void)snprintf(logincmd, sizeof(logincmd), "%s %d %u %s",
+ loginfile, unit, speed, loginargs);
+
/*
* aim stdout and errout at /dev/null so logincmd output won't
* babble into the slip tty line.
*/
- (void) close(1);
+ (void)close(1);
if ((fd = open(_PATH_DEVNULL, O_WRONLY)) != 1) {
if (fd < 0) {
syslog(LOG_ERR, "open %s: %m", _PATH_DEVNULL);
exit(1);
}
- (void) dup2(fd, 1);
- (void) close(fd);
+ (void)dup2(fd, 1);
+ (void)close(fd);
}
- (void) dup2(1, 2);
+ (void)dup2(1, 2);
/*
* Run login and logout scripts as root (real and effective);
* current route(8) is setuid root, and checks the real uid
* to see whether changes are allowed (or just "route get").
*/
- (void) setuid(0);
- if (s = system(logincmd)) {
+ (void)setuid(0);
+ if ((s = system(logincmd)) != 0) {
syslog(LOG_ERR, "%s login failed: exit status %d from %s",
- loginname, s, loginfile);
+ loginname, s, loginfile);
exit(6);
}
@@ -539,6 +547,5 @@
/* twiddle thumbs until we get a signal */
while (1)
sigpause(0);
-
/* NOTREACHED */
}
[-- Attachment #3 --]
--- sliplogin.c.1 Sat Sep 15 21:21:22 2001
+++ sliplogin.c Sat Sep 15 22:19:16 2001
@@ -124,6 +124,8 @@
static char iffilename[MAXPATHLEN]; /* name of if file */
static pid_t pid; /* our pid */
+volatile sig_atomic_t sig_hup;
+
char *
make_ipaddr(void)
{
@@ -158,15 +160,14 @@
environ = restricted_environ; /* minimal protection for system() */
- (void)strncpy(loginname, name, sizeof(loginname)-1);
- loginname[sizeof(loginname)-1] = '\0';
+ (void)strlcpy(loginname, name, sizeof(loginname));
if ((fp = fopen(_PATH_ACCESS, "r")) == NULL) {
accfile_err:
syslog(LOG_ERR, "%s: %m\n", _PATH_ACCESS);
exit(1);
}
- while (fgets(loginargs, sizeof(loginargs) - 1, fp)) {
+ while (fgets(loginargs, sizeof(loginargs), fp)) {
if (ferror(fp))
goto accfile_err;
if (loginargs[0] == '#' || isspace(loginargs[0]))
@@ -206,9 +207,8 @@
(void)snprintf(loginfile, sizeof(loginfile),
"%s.%s", _PATH_LOGIN, name);
if (access(loginfile, R_OK|X_OK) != 0) {
- (void)strncpy(loginfile, _PATH_LOGIN,
- sizeof(loginfile)-1);
- loginfile[sizeof(loginfile)-1] = '\0';
+ (void)strlcpy(loginfile, _PATH_LOGIN,
+ sizeof(loginfile));
if (access(loginfile, R_OK|X_OK)) {
syslog(LOG_ERR,
"access denied for %s - no %s\n",
@@ -219,9 +219,8 @@
(void)snprintf(slparmsfile, sizeof(slparmsfile),
"%s.%s", _PATH_SLPARMS, name);
if (access(slparmsfile, R_OK|X_OK) != 0) {
- (void)strncpy(slparmsfile, _PATH_SLPARMS,
- sizeof(slparmsfile)-1);
- slparmsfile[sizeof(slparmsfile)-1] = '\0';
+ (void)strlcpy(slparmsfile, _PATH_SLPARMS,
+ sizeof(slparmsfile));
if (access(slparmsfile, R_OK|X_OK))
*slparmsfile = '\0';
}
@@ -234,7 +233,7 @@
exit(1);
}
n = 0;
- while (fgets(buf, sizeof(buf) - 1, fp) != NULL) {
+ while (fgets(buf, sizeof(buf), fp) != NULL) {
if (ferror(fp))
goto slfile_err;
if (buf[0] == '#' || isspace(buf[0]))
@@ -306,20 +305,26 @@
}
void
+wanthup(s)
+ int s;
+{
+ sig_hup = s;
+}
+
+void
hup_handler(s)
int s;
{
char logoutfile[MAXPATHLEN];
char logincmd[2*MAXPATHLEN+32];
+ sig_hup = 0;
(void)close(0);
seteuid(0);
(void)snprintf(logoutfile, sizeof(logoutfile),
"%s.%s", _PATH_LOGOUT, loginname);
- if (access(logoutfile, R_OK|X_OK) != 0) {
- (void)strncpy(logoutfile, _PATH_LOGOUT, sizeof(logoutfile)-1);
- logoutfile[sizeof(logoutfile)-1] = '\0';
- }
+ if (access(logoutfile, R_OK|X_OK) != 0)
+ (void)strlcpy(logoutfile, _PATH_LOGOUT, sizeof(logoutfile));
if (access(logoutfile, R_OK|X_OK) == 0) {
(void)snprintf(logincmd, sizeof(logincmd),
"%s %d %u %s", logoutfile, unit, speed, loginargs);
@@ -461,11 +466,11 @@
syslog(LOG_ERR, "ioctl (SLIOCGUNIT): %m");
exit(1);
}
- (void)signal(SIGHUP, hup_handler);
- (void)signal(SIGTERM, hup_handler);
+ (void)signal(SIGHUP, wanthup);
+ (void)signal(SIGTERM, wanthup);
if (keepal > 0) {
- (void) signal(SIGURG, hup_handler);
+ (void)signal(SIGURG, wanthup);
if (ioctl(0, SLIOCSKEEPAL, &keepal) < 0) {
syslog(LOG_ERR, "ioctl(SLIOCSKEEPAL): %m");
exit(1);
@@ -545,7 +550,10 @@
/* reset uid to users' to allow the user to give a signal. */
seteuid(uid);
/* twiddle thumbs until we get a signal */
- while (1)
+ while (1) {
+ if (sig_hup)
+ hup_handler(sig_hup);
sigpause(0);
+ }
/* NOTREACHED */
}
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1010915232244.41506A-300000>