at(1) tries to run as little code as possible with privileges. This
creates a false sense of security since if there is an overflow an
attacker can easily change the effective gid anyway.
The only place we really need to drop the setgid crontab is when
reading a file with the -f flag.
- todd
Index: usr.bin/at/at.c
===================================================================
RCS file: /cvs/src/usr.bin/at/at.c,v
retrieving revision 1.66
diff -u -p -u -r1.66 at.c
--- usr.bin/at/at.c 28 Oct 2015 20:17:31 -0000 1.66
+++ usr.bin/at/at.c 2 Nov 2015 21:16:09 -0000
@@ -35,7 +35,6 @@
#include "cron.h"
#include "at.h"
-#include "privs.h"
#include <limits.h>
#define ALARMC 10 /* Number of seconds to wait for timeout */
@@ -56,6 +55,9 @@ char vflag = 0; /* show completed but
char force = 0; /* suppress errors (atrm) */
char interactive = 0; /* interactive mode (atrm) */
static int send_mail = 0; /* whether we are sending mail */
+uid_t user_uid; /* user's real uid */
+gid_t user_gid; /* user's real gid */
+gid_t spool_gid; /* gid for writing to at spool */
static void sigc(int);
static void alarmc(int);
@@ -77,11 +79,8 @@ static __dead void
panic(const char *a)
{
(void)fprintf(stderr, "%s: %s\n", ProgramName, a);
- if (fcreated) {
- PRIV_START;
+ if (fcreated)
unlink(atfile);
- PRIV_END;
- }
exit(EXIT_FAILURE);
}
@@ -93,11 +92,8 @@ static __dead void
panic2(const char *a, const char *b)
{
(void)fprintf(stderr, "%s: %s%s\n", ProgramName, a, b);
- if (fcreated) {
- PRIV_START;
+ if (fcreated)
unlink(atfile);
- PRIV_END;
- }
exit(EXIT_FAILURE);
}
@@ -110,11 +106,8 @@ perr(const char *a)
{
if (!force)
perror(a);
- if (fcreated) {
- PRIV_START;
+ if (fcreated)
unlink(atfile);
- PRIV_END;
- }
exit(EXIT_FAILURE);
}
@@ -135,11 +128,8 @@ static void
sigc(int signo)
{
/* If the user presses ^C, remove the spool file and exit. */
- if (fcreated) {
- PRIV_START;
+ if (fcreated)
(void)unlink(atfile);
- PRIV_END;
- }
_exit(EXIT_FAILURE);
}
@@ -204,8 +194,6 @@ writefile(const char *cwd, time_t runtim
act.sa_flags = 0;
sigaction(SIGINT, &act, NULL);
- PRIV_START;
-
if ((lockdes = open(AT_DIR, O_RDONLY, 0)) < 0)
perr("Cannot open jobs dir");
@@ -241,11 +229,9 @@ writefile(const char *cwd, time_t runtim
if ((fd2 = dup(fdes)) < 0)
perr("Error in dup() of job file");
- if (fchown(fd2, real_uid, real_gid) != 0)
+ if (fchown(fd2, user_uid, user_gid) != 0)
perr("Cannot give away file");
- PRIV_END;
-
/*
* We've successfully created the file; let's set the flag so it
* gets removed in case of an interrupt or error.
@@ -268,7 +254,7 @@ writefile(const char *cwd, time_t runtim
if ((mailname == NULL) || (mailname[0] == '\0') ||
(strlen(mailname) > MAX_UNAME) || (getpwnam(mailname) == NULL)) {
- pass_entry = getpwuid(real_uid);
+ pass_entry = getpwuid(user_uid);
if (pass_entry != NULL)
mailname = pass_entry->pw_name;
}
@@ -279,7 +265,7 @@ writefile(const char *cwd, time_t runtim
* that, /bin/sh.
*/
if ((shell = getenv("SHELL")) == NULL || *shell == '\0') {
- pass_entry = getpwuid(real_uid);
+ pass_entry = getpwuid(user_uid);
if (pass_entry != NULL && *pass_entry->pw_shell != '\0')
shell = pass_entry->pw_shell;
else
@@ -287,7 +273,7 @@ writefile(const char *cwd, time_t runtim
}
(void)fprintf(fp, "#!/bin/sh\n# atrun uid=%lu gid=%lu\n# mail %*s %d\n",
- (unsigned long)real_uid, (unsigned long)real_gid,
+ (unsigned long)user_uid, (unsigned long)user_gid,
MAX_UNAME, mailname, send_mail);
/* Write out the umask at the time of invocation */
@@ -394,9 +380,7 @@ writefile(const char *cwd, time_t runtim
(void)close(fd2);
/* Poke cron so it knows to reload the at spool. */
- PRIV_START;
poke_daemon(AT_DIR, RELOAD_AT);
- PRIV_END;
runtime = *localtime(&runtimer);
strftime(timestr, TIMESIZE, "%a %b %e %T %Y", &runtime);
@@ -485,7 +469,7 @@ list_jobs(int argc, char **argv, int cou
for (i = 0; i < argc; i++) {
if ((pw = getpwnam(argv[i])) == NULL)
panic2(argv[i], ": invalid user name");
- if (pw->pw_uid != real_uid && real_uid != 0)
+ if (pw->pw_uid != user_uid && user_uid != 0)
panic("Only the superuser may display other
users' jobs");
uids[i] = pw->pw_uid;
}
@@ -494,16 +478,12 @@ list_jobs(int argc, char **argv, int cou
shortformat = strcmp(ProgramName, "at") == 0;
- PRIV_START;
-
if (chdir(AT_DIR) != 0)
perr2("Cannot change to ", AT_DIR);
if ((spool = opendir(".")) == NULL)
perr2("Cannot open ", AT_DIR);
- PRIV_END;
-
if (fstat(dirfd(spool), &stbuf) != 0)
perr2("Cannot stat ", AT_DIR);
@@ -520,19 +500,15 @@ list_jobs(int argc, char **argv, int cou
/* Loop over every file in the directory. */
while ((dirent = readdir(spool)) != NULL) {
- PRIV_START;
-
if (stat(dirent->d_name, &stbuf) != 0)
perr2("Cannot stat in ", AT_DIR);
- PRIV_END;
-
/*
* See it's a regular file and has its x bit turned on and
* is the user's
*/
if (!S_ISREG(stbuf.st_mode)
- || ((stbuf.st_uid != real_uid) && !(real_uid == 0))
+ || ((stbuf.st_uid != user_uid) && !(user_uid == 0))
|| !(S_IXUSR & stbuf.st_mode || vflag))
continue;
@@ -638,16 +614,12 @@ process_jobs(int argc, char **argv, int
int job_matches, jobs_len, uids_len;
int error, i, ch, changed;
- PRIV_START;
-
if (chdir(AT_DIR) != 0)
perr2("Cannot change to ", AT_DIR);
if ((spool = opendir(".")) == NULL)
perr2("Cannot open ", AT_DIR);
- PRIV_END;
-
/* Convert argv into a list of jobs and uids. */
jobs = NULL;
uids = NULL;
@@ -663,7 +635,7 @@ process_jobs(int argc, char **argv, int
*(ep + 2) == '\0' && l > 0 && l < INT_MAX)
jobs[jobs_len++] = argv[i];
else if ((pw = getpwnam(argv[i])) != NULL) {
- if (real_uid != pw->pw_uid && real_uid != 0) {
+ if (user_uid != pw->pw_uid && user_uid != 0) {
fprintf(stderr, "%s: Only the superuser"
" may %s other users' jobs\n",
ProgramName, what == ATRM
@@ -679,13 +651,10 @@ process_jobs(int argc, char **argv, int
/* Loop over every file in the directory */
changed = 0;
while ((dirent = readdir(spool)) != NULL) {
-
- PRIV_START;
if (stat(dirent->d_name, &stbuf) != 0)
perr2("Cannot stat in ", AT_DIR);
- PRIV_END;
- if (stbuf.st_uid != real_uid && real_uid != 0)
+ if (stbuf.st_uid != user_uid && user_uid != 0)
continue;
if (strtot(dirent->d_name, &ep, &runtimer) == -1)
@@ -718,8 +687,6 @@ process_jobs(int argc, char **argv, int
if (job_matches) {
switch (what) {
case ATRM:
- PRIV_START;
-
if (!interactive ||
(interactive && rmok(runtimer))) {
if (unlink(dirent->d_name) == 0)
@@ -731,18 +698,10 @@ process_jobs(int argc, char **argv, int
"%s removed\n",
dirent->d_name);
}
-
- PRIV_END;
-
break;
case CAT:
- PRIV_START;
-
fp = fopen(dirent->d_name, "r");
-
- PRIV_END;
-
if (!fp)
perr("Cannot open file");
@@ -773,12 +732,10 @@ process_jobs(int argc, char **argv, int
/* If we modied the spool, poke cron so it knows to reload. */
if (changed) {
- PRIV_START;
if (chdir(CRONDIR) != 0)
perror(CRONDIR);
else
poke_daemon(AT_DIR, RELOAD_AT);
- PRIV_END;
}
return (error);
@@ -870,22 +827,14 @@ ttime(char *arg)
static int
check_permission(void)
{
- int ok;
- uid_t uid = geteuid();
struct passwd *pw;
- if ((pw = getpwuid(uid)) == NULL) {
+ if ((pw = getpwuid(user_uid)) == NULL) {
perror("Cannot access password database");
exit(EXIT_FAILURE);
}
- PRIV_START;
-
- ok = allowed(pw->pw_name, AT_ALLOW, AT_DENY);
-
- PRIV_END;
-
- return (ok);
+ return (allowed(pw->pw_name, AT_ALLOW, AT_DENY));
}
static __dead void
@@ -942,7 +891,9 @@ main(int argc, char **argv)
else
ProgramName = argv[0];
- RELINQUISH_PRIVS;
+ user_uid = getuid();
+ user_gid = getgid();
+ spool_gid = getegid();
/* find out what this program is supposed to do */
if (strcmp(ProgramName, "atq") == 0) {
@@ -1046,8 +997,12 @@ main(int argc, char **argv)
case AT:
case BATCH:
if (atinput != NULL) {
+ if (setegid(user_gid) != 0)
+ perr("setegid(user_gid)");
if (freopen(atinput, "r", stdin) == NULL)
perr("Cannot open input file");
+ if (setegid(spool_gid) != 0)
+ perr("setegid(spool_gid)");
}
break;
default:
Index: usr.bin/at/privs.h
===================================================================
RCS file: usr.bin/at/privs.h
diff -N usr.bin/at/privs.h
--- usr.bin/at/privs.h 2 Jul 2010 23:40:09 -0000 1.10
+++ /dev/null 1 Jan 1970 00:00:00 -0000
@@ -1,131 +0,0 @@
-/* $OpenBSD: privs.h,v 1.10 2010/07/02 23:40:09 krw Exp $ */
-
-/*
- * privs.h - header for privileged operations
- * Copyright (C) 1993 Thomas Koenig
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. The name of the author(s) may not be used to endorse or promote
- * products derived from this software without specific prior written
- * permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _PRIVS_H
-#define _PRIVS_H
-
-#include <unistd.h>
-
-/* Relinquish privileges temporarily for a setuid or setgid program
- * with the option of getting them back later. This is done by
- * utilizing POSIX saved user and groups ids (or setreuid amd setregid if
- * POSIX saved ids are not available). Call RELINQUISH_PRIVS once
- * at the beginning of the main program. This will cause all operations
- * to be executed with the real userid. When you need the privileges
- * of the setuid/setgid invocation, call PRIV_START; when you no longer
- * need it, call PRIV_END. Note that it is an error to call PRIV_START
- * and not PRIV_END within the same function.
- *
- * Use RELINQUISH_PRIVS_ROOT(a,b) if your program started out running
- * as root, and you want to drop back the effective userid to a
- * and the effective group id to b, with the option to get them back
- * later.
- *
- * Problems: Do not use return between PRIV_START and PRIV_END; this
- * will cause the program to continue running in an unprivileged
- * state.
- *
- * It is NOT safe to call exec(), system() or popen() with a user-
- * supplied program (i.e. without carefully checking PATH and any
- * library load paths) with relinquished privileges; the called program
- * can acquire them just as easily. Set both effective and real userid
- * to the real userid before calling any of them.
- */
-
-#ifndef MAIN_PROGRAM
-extern
-#endif
-uid_t real_uid, effective_uid;
-
-#ifndef MAIN_PROGRAM
-extern
-#endif
-gid_t real_gid, effective_gid;
-
-#ifdef HAVE_SAVED_UIDS
-
-#define RELINQUISH_PRIVS do { \
- real_uid = getuid(); \
- effective_uid = geteuid(); \
- real_gid = getgid(); \
- effective_gid = getegid(); \
- setegid(real_gid); \
- seteuid(real_uid); \
-} while (0)
-
-#define RELINQUISH_PRIVS_ROOT(a, b) do { \
- real_uid = (a); \
- effective_uid = geteuid(); \
- real_gid = (b); \
- effective_gid = getegid(); \
- setegid(real_gid); \
- seteuid(real_uid); \
-} while (0)
-
-#define PRIV_START do { \
- seteuid(effective_uid); \
- setegid(effective_gid); \
-} while (0)
-
-#define PRIV_END do { \
- setegid(real_gid); \
- seteuid(real_uid); \
-} while (0)
-
-#else /* HAVE_SAVED_UIDS */
-
-#define RELINQUISH_PRIVS do { \
- real_uid = getuid(); \
- effective_uid = geteuid(); \
- real_gid = getgid(); \
- effective_gid = getegid(); \
- setregid(effective_gid, real_gid); \
- setreuid(effective_uid, real_uid); \
-} while (0)
-
-#define RELINQUISH_PRIVS_ROOT(a, b) do { \
- real_uid = (a); \
- effective_uid = geteuid(); \
- real_gid = (b); \
- effective_gid = getegid(); \
- setregid(effective_gid, real_gid); \
- setreuid(effective_uid, real_uid); \
-} while (0)
-
-#define PRIV_START do { \
- setreuid(real_uid, effective_uid); \
- setregid(real_gid, effective_gid); \
-} while (0)
-
-#define PRIV_END do { \
- setregid(effective_gid, real_gid); \
- setreuid(effective_uid, real_uid); \
-} while (0)
-
-#endif /* HAVE_SAVED_UIDS */
-
-#endif /* _PRIVS_H */
