On 2016/02/22 15:09, Theo de Raadt wrote:
> > RCS file:
> > /build/data/openbsd/cvs/src/usr.sbin/pkg_add/OpenBSD/PackageRepository.pm,v
> > retrieving revision 1.117
> > diff -u -p -r1.117 PackageRepository.pm
> > --- OpenBSD/PackageRepository.pm 9 Feb 2016 10:02:27 -0000 1.117
> > +++ OpenBSD/PackageRepository.pm 22 Feb 2016 21:59:35 -0000
> > @@ -586,8 +586,14 @@ sub drop_privileges_and_setup_env
> > $< = $uid;
> > $> = $uid;
> > }
> > - $ENV{LC_ALL} = 'C';
> > # don't error out yet if we can't change.
> > +
> > + # proper error messages
> > + $ENV{LC_ALL} = 'C';
> > + # sanitize env for ftp
> > + delete $ENV{HOME};
> > + delete $ENV{PAGER};
> > + delete $ENV{TMPDIR};
> > }
>
> I am not sure whether this approach is sufficient.
>
> The situation here is that a process is being started (fork + exec) on
> the other side of a priv boundary.
>
> The general mode of handling environment should be:
>
> 1. sanitize is
> 2. consider thinking about this as a white-list, rather than a blacklist
> 3. that process will happily parse all env variables, since there is no
> issetugid in effect
>
> I'd like to propose
>
> 0. start with an empty environment
> 1. pass LOGNAME and USER unmolested
> 2. force PATH to the canonical default
> 3. pass SHELL unmolested, or force it to /bin/ksh
> 4. set HOME to /var/empty (no $HOME is a rare situation for programs to
> handle)
>
> You are not just satisfying the ftp binary, but also the libc it is
> using. Maybe you want to also pass some LANG type things, not sure.
If using that approach, ftp's variables need to be passed too,
at least FTPMODE, ftp_proxy, http_proxy, and if people are using
an FETCH_CMD other than ftp (some people need curl for ntlm auth
against certain proxies) they might need https_proxy, no_proxy,
all_proxy.
