Anyone? We need to do something because the manual is currently wrong.
On 2013/03/28 15:27, Stuart Henderson wrote: > On 2013/03/23 19:37, patrick keshishian wrote: > > On Sat, Mar 23, 2013 at 5:15 PM, Creamy <[email protected]> wrote: > > > From the pppd man page: > > > > > > 1163:.Sh SCRIPTS > > > 1164-.Nm > > > 1165-invokes scripts at various stages in its processing which can be > > > 1166-used to perform site-specific ancillary processing. > > > 1167-These scripts are usually shell scripts, but could be executable > > > code files > > > 1168-instead. > > > 1169-.Nm > > > 1170-does not wait for the scripts to finish. > > > 1171-The scripts are executed as root (with the real and effective user > > > ID set to 0), > > > ^^^^^^^^^^^^^^^^^^^^ > > > > > > No, they are not. Which is wrong, the man page, or the behavior of pppd? > > > > some history: http://marc.info/?l=openbsd-tech&m=123494734721801 > > > > ... from 2009. yikes :) > > > > --patrick > > > > I think this diff is in the right direction, but that we should > check permissions on the scripts to make sure they aren't group/world > writable. Of course it doesn't stop people shooting themselves in > the foot by running a writable script from another, but stops > accidental problems. > > Any OKs ? > > Index: main.c > =================================================================== > RCS file: /cvs/src/usr.sbin/pppd/main.c,v > retrieving revision 1.49 > diff -u -p -r1.49 main.c > --- main.c 30 Apr 2011 18:49:38 -0000 1.49 > +++ main.c 28 Mar 2013 15:25:56 -0000 > @@ -1173,9 +1173,17 @@ run_program(prog, args, must_exist) > char **args; > int must_exist; > { > + struct stat sbuf; > pid_t pid; > uid_t uid; > - gid_t gid; > + > + if (stat(prog, &sbuf) < 0) { > + syslog(LOG_ERR, "cannot stat program file %s: %m", prog); > + return -1; > + } else if ((sbuf.st_mode & (S_IWGRP | S_IWOTH)) != 0) { > + syslog(LOG_ERR, "%s: group/world writable", prog); > + return -1; > + } > > pid = fork(); > if (pid == -1) { > @@ -1190,10 +1198,9 @@ run_program(prog, args, must_exist) > (void) umask (S_IRWXG|S_IRWXO); > (void) chdir ("/"); /* no current directory. */ > > - /* revoke privs */ > + /* Execute scripts as root, to allow routing table changes etc. */ > uid = getuid(); > - gid = getgid(); > - if (setresgid(gid, gid, gid) == -1 || setresuid(uid, uid, uid) == -1) { > + if (setreuid(0, 0) == -1) { > syslog(LOG_ERR, "revoke privileges: %s", strerror(errno)); > _exit(1); > } >
