On Tue, Apr 09, 2013 at 10:06:42AM +0100, Stuart Henderson wrote: > Anyone? > > We need to do something because the manual is currently wrong. >
ok! jmc > > 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); > > } > > >
