Re: SUID scripts?

Mike (ford@omnicron.com)
Thu, 6 Aug 98 17:23:38 -0700


> From: Christopher Curtis <ccurtis@ee.fit.edu>
>
> I'm trying to write a SUID shellscript, but am having problems.

> From: Jeff Sturm <jsturm@gatecom.com>
>
> Setuid shell scripts are considered dangerous. I don't think they work
> on any Linux platform. Alternatively, you can use perl or a C wrapper
> binary (but beware of environment variables, if security is an issue).

Here is an implementation of "secure" set-ID #! files (A.K.A. set-uid
shell scripts) for Linux 2.0.x. I have been running it for over a year
but as you can see, it still has a printk or two which reveal that I
don't actually run very many set-ID shell scripts (the printk's would
get annoying). You can remove the printk's or leave them in if you're
paranoid or want to know when someone is probing for a security hole in
your system.

This patch implements set-ID #! shell scripts and makes them as secure
as they are under Solaris, Unix SVR4, etc. The major hole that would
otherwise make set-ID scripts insecure (replacement of the file
inbetween the exec and the shell calling open()) is eliminated by
changing the shell's argv[1] to the name of a file descriptor to the
script file opened by the kernel during exec(). There can, of course,
be other security holes due to the contents of your script. Be careful.

Also included in this patch is a fix for a bug in /proc which
prevented a process from examining itself under some conditions.

I'm running 2.0.33 with this patch. I haven't yet looked to see how much
would have to change for 2.1.x.
-=] Ford [=-

"I can stand the sight of worms (In Real Life: Mike Ditto)
or look at microscopic germs ford@omnicron.com
but technicolor pachyderms http://www.omnicron.com/~ford/ford.html
are really too much for me."

--- include/linux/proc_fs.h.orig Wed Apr 8 15:38:22 1998
+++ include/linux/proc_fs.h Thu Aug 6 16:58:52 1998
@@ -256,6 +256,7 @@
extern int init_proc_fs(void);
extern struct inode * proc_get_inode(struct super_block *, int, struct proc_dir_entry *);
extern void proc_statfs(struct super_block *, struct statfs *, int);
+extern int proc_permission(struct inode *, int);
extern void proc_read_inode(struct inode *);
extern void proc_write_inode(struct inode *);
extern int proc_match(int, const char *, struct proc_dir_entry *);
--- fs/proc/link.c.orig Sat Nov 1 12:31:42 1997
+++ fs/proc/link.c Thu Aug 6 16:45:25 1998
@@ -58,7 +58,7 @@
NULL, /* writepage */
NULL, /* bmap */
NULL, /* truncate */
- NULL /* permission */
+ proc_permission /* permission */
};


--- fs/proc/fd.c.orig Wed Nov 12 20:36:41 1997
+++ fs/proc/fd.c Thu Aug 6 16:45:25 1998
@@ -49,7 +49,7 @@
NULL, /* writepage */
NULL, /* bmap */
NULL, /* truncate */
- NULL /* permission */
+ proc_permission /* permission */
};

static int proc_lookupfd(struct inode * dir, const char * name, int len,
--- fs/proc/inode.c.orig Sat Nov 1 12:31:17 1997
+++ fs/proc/inode.c Thu Aug 6 16:52:26 1998
@@ -129,6 +129,44 @@
memcpy_tofs(buf, &tmp, bufsiz);
}

+int proc_permission(struct inode * inode, int mask)
+{
+ unsigned long ino, pid;
+ struct task_struct * p;
+ int i, mode;
+
+ ino = inode->i_ino;
+ pid = ino >> 16;
+ p = task[0];
+ for (i = 0; i < NR_TASKS ; i++)
+ if ((p = task[i]) && (p->pid == pid))
+ break;
+ if (!p || i >= NR_TASKS)
+ return -ENOENT;
+
+ /* If the process is trying to examine itself, allow any access. */
+ if (current == p ||
+ (current->euid == p->euid && current->uid == p->uid &&
+ current->egid == p->egid && current->gid == p->gid))
+ return 0;
+
+ /* Otherwise just duplicate what permission() in namei.c would do. */
+ mode = inode->i_mode;
+ if ((mask & S_IWOTH) && IS_RDONLY(inode) &&
+ (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode)))
+ return -EROFS; /* Nobody gets write access to a read-only fs */
+ else if ((mask & S_IWOTH) && IS_IMMUTABLE(inode))
+ return -EACCES; /* Nobody gets write access to an immutable file */
+ else if (current->fsuid == inode->i_uid)
+ mode >>= 6;
+ else if (in_group_p(inode->i_gid))
+ mode >>= 3;
+ if (((mode & mask & 0007) == mask) || fsuser())
+ return 0;
+ return -EACCES;
+}
+
+
void proc_read_inode(struct inode * inode)
{
unsigned long ino, pid;
--- fs/exec.c.orig Wed Dec 10 18:09:44 1997
+++ fs/exec.c Thu Aug 6 13:33:30 1998
@@ -522,8 +522,6 @@
if (bprm->inode->i_writecount > 0)
return -ETXTBSY;

- bprm->e_uid = current->euid;
- bprm->e_gid = current->egid;
id_change = 0;

/* Set-uid? */
@@ -676,6 +674,8 @@
if ((bprm.envc = count(envp)) < 0)
return bprm.envc;

+ bprm.e_uid = current->euid;
+ bprm.e_gid = current->egid;
retval = prepare_binprm(&bprm);

if(retval>=0) {
--- fs/binfmt_script.c.orig Sat Mar 23 04:46:40 1996
+++ fs/binfmt_script.c Thu Aug 6 16:21:08 1998
@@ -14,7 +14,8 @@
static int do_load_script(struct linux_binprm *bprm,struct pt_regs *regs)
{
char *cp, *interp, *i_name, *i_arg;
- int retval;
+ int retval, script_fd;
+ char buf[20];
if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') || (bprm->sh_bang))
return -ENOEXEC;
/*
@@ -23,7 +24,9 @@
*/

bprm->sh_bang++;
- iput(bprm->inode);
+ /* dont_iput prevents other binfmt types from examining buf after we
+ modify it here, but we have to make sure to do iput in any early
+ error return cases. */
bprm->dont_iput=1;

bprm->buf[127] = '\0';
@@ -38,8 +41,10 @@
break;
}
for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
- if (!cp || *cp == '\0')
+ if (!cp || *cp == '\0') {
+ iput(bprm->inode);
return -ENOEXEC; /* No interpreter name found */
+ }
interp = i_name = cp;
i_arg = 0;
for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++) {
@@ -51,11 +56,60 @@
if (*cp)
i_arg = cp;
/*
- * OK, we've parsed out the interpreter name and
- * (optional) argument.
+ * OK, we've parsed out the interpreter name and (optional) argument.
+ * Now open a file descriptor to the script file if necessary for
+ * secure set-id purposes.
+ */
+ if (bprm->e_uid != current->euid ||
+ bprm->e_gid != current->egid) {
+ struct file *f;
+ int error;
+ f = get_empty_filp();
+ if (!f) {
+ iput(bprm->inode);
+ return -ENFILE;
+ }
+
+ f->f_flags = 0;
+ f->f_mode = FMODE_READ;
+ f->f_inode = bprm->inode;
+ f->f_pos = 0;
+ f->f_reada = 0;
+ f->f_op = NULL;
+ if (f->f_inode->i_op)
+ f->f_op = f->f_inode->i_op->default_file_ops;
+ if (f->f_op && f->f_op->open) {
+ error = f->f_op->open(f->f_inode, f);
+ if (error) {
+ f->f_count--;
+ iput(bprm->inode);
+ return error;
+ }
+ }
+
+ script_fd = get_unused_fd();
+ if (script_fd < 0) {
+ close_fp(f);
+ return script_fd;
+ }
+ current->files->fd[script_fd] = f;
+ sprintf(buf, "/dev/fd/%d", script_fd);
+ bprm->filename = buf;
+
+printk("set-ID interpreter exec: ");
+if (i_arg)
+printk("intp=%s, arg1=%s, arg2=%s\n", i_name, i_arg, bprm->filename);
+else
+printk("intp=%s, arg1=%s\n", i_name, bprm->filename);
+ } else {
+ iput(bprm->inode);
+ script_fd = -1;
+ }
+
+ /*
* Splice in (1) the interpreter's name for argv[0]
* (2) (optional) argument to interpreter
- * (3) filename of shell script (replace argv[0])
+ * (3) filename of script or /dev/fd/N (replace argv[0])
*
* This is done in reverse order, because of how the
* user environment and arguments are stored.
@@ -69,21 +123,51 @@
}
bprm->p = copy_strings(1, &i_name, bprm->page, bprm->p, 2);
bprm->argc++;
- if (!bprm->p)
- return -E2BIG;
+ if (!bprm->p) {
+ if (script_fd>=0) {
+ struct file *f = current->files->fd[script_fd];
+ current->files->fd[script_fd] = 0;
+ put_unused_fd(script_fd);
+ close_fp(f);
+ }
+ return -E2BIG;
+ }
+
/*
* OK, now restart the process with the interpreter's inode.
* Note that we use open_namei() as the name is now in kernel
* space, and we don't need to copy it.
*/
retval = open_namei(interp, 0, 0, &bprm->inode, NULL);
- if (retval)
- return retval;
- bprm->dont_iput=0;
- retval=prepare_binprm(bprm);
- if(retval<0)
- return retval;
- return search_binary_handler(bprm,regs);
+ if (retval) {
+ if (script_fd>=0) {
+ struct file *f = current->files->fd[script_fd];
+ current->files->fd[script_fd] = 0;
+ put_unused_fd(script_fd);
+ close_fp(f);
+ }
+ return retval;
+ }
+ bprm->dont_iput = 0;
+ retval = prepare_binprm(bprm);
+ if (retval<0) {
+ if (script_fd>=0) {
+ struct file *f = current->files->fd[script_fd];
+ current->files->fd[script_fd] = 0;
+ put_unused_fd(script_fd);
+ close_fp(f);
+ }
+ return retval;
+ }
+
+ retval = search_binary_handler(bprm, regs);
+ if (retval<0 && script_fd>=0) {
+ struct file *f = current->files->fd[script_fd];
+ current->files->fd[script_fd] = 0;
+ put_unused_fd(script_fd);
+ close_fp(f);
+ }
+ return retval;
}

static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html