From 59f147b0d9bfb46577cd6f8813589ccdf8bc2a2c Mon Sep 17 00:00:00 2001 From: mdw Date: Mon, 29 Jun 1998 13:10:41 +0000 Subject: [PATCH] Add some commentary regarding an issue in `sudo' which affects `become'; I'm not fixing it yet because I don't think it's important. Fixed the PATH lookup code to use the right binary name: `binary' rather than `todo[0]'. The two only differ when `style' is `l_login', in which case `binary' has an initial `/' anyway, and the PATH lookup code is never invoked. The name is used in a buffer-overflow precheck, though, and auditing is easier if the naming is consistent. --- src/become.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/become.c b/src/become.c index c5df906..23f69dd 100644 --- a/src/become.c +++ b/src/become.c @@ -1,6 +1,6 @@ /* -*-c-*- * - * $Id: become.c,v 1.18 1998/06/26 10:32:54 mdw Exp $ + * $Id: become.c,v 1.19 1998/06/29 13:10:41 mdw Exp $ * * Main code for `become' * @@ -29,6 +29,16 @@ /*----- Revision history --------------------------------------------------* * * $Log: become.c,v $ + * Revision 1.19 1998/06/29 13:10:41 mdw + * Add some commentary regarding an issue in `sudo' which affects `become'; + * I'm not fixing it yet because I don't think it's important. + * + * Fixed the PATH lookup code to use the right binary name: `binary' rather + * than `todo[0]'. The two only differ when `style' is `l_login', in which + * case `binary' has an initial `/' anyway, and the PATH lookup code is + * never invoked. The name is used in a buffer-overflow precheck, though, + * and auditing is easier if the naming is consistent. + * * Revision 1.18 1998/06/26 10:32:54 mdw * Cosmetic change: use sizeof(destination) in memcpy. * @@ -1376,9 +1386,20 @@ done_options: if (strlen(p) + strlen(binary) + 2 > sizeof(rq.cmd)) continue; - /* --- Now build the pathname and check it --- */ + /* --- Now build the pathname and check it --- * + * + * Issue: user can take advantage of these privileges to decide whether + * a program with a given name exists. I'm not sure that's + * particularly significant: it only works on regular files with + * execute permissions, and if you're relying on the names of these + * being secret to keep your security up, then you're doing something + * deeply wrong anyway. On the other hand, it's useful to allow people + * to be able to execute programs and scripts which they wouldn't + * otherwise have access to. [This problem was brought up on + * Bugtraq, as a complaint against sudo.] + */ - sprintf(rq.cmd, "%s/%s", p, todo[0]); + sprintf(rq.cmd, "%s/%s", p, binary); if (stat(rq.cmd, &st) == 0 && /* Check it exists */ st.st_mode & 0111 && /* Check it's executable */ S_ISREG(st.st_mode)) /* Check it's a file */ -- 2.11.0