Date: Thu, 01 Aug 2002 19:06:43 +0300 From: Maxim Sobolev <sobomax@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au>, current@FreeBSD.org, obrien@FreeBSD.org Subject: Re: pkg_add broken by POLA breakage in tar Message-ID: <3D495C93.C7198139@FreeBSD.org> References: <20020801203312.V1911-100000@gamplex.bde.org> <3D495A26.4627C170@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Maxim Sobolev wrote:
>
> Bruce Evans wrote:
> >
> > Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably
> > thousands of user scripts that are no more careful than pkg_add) in
> > -current and RELENG_4:
>
> Are you sure? My own investigation at the time of the commit showed
> that old tar shipped with FreeBSD, was adjusting permissions of
> extracting files when running as uid 0 according to current umask
> settings, so that IMO 1.2-1.3 actually restored POLA, not broke it.
Need evidence? Here it is:
# cvs co -D "10 months ago" src/gnu/usr.bin/tar
[...]
# cd src/gnu/usr.bin/tar
# make
[...]
# mkdir foo
# touch foo/bar
# chmod 777 foo
# chmod 777 foo/bar
# ./tar cvf foo.tar foo
foo/
foo/bar
# rm -rf foo
# ./tar xvf foo.tar
foo/
foo/bar
root@notebook# ls -l | grep foo
drwxr-xr-x 2 root wheel 512 1 âþú 19:01 foo/
-rw-r--r-- 1 root wheel 10240 1 âþú 19:01 foo.tar
root@notebook# ls -l foo
total 0
-rwxr-xr-x 1 root wheel 0 1 âþú 19:01 bar*
# umask
0022
#
-Maxim
>
> -Maxim
>
> >
> > % RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
> > % Working file: extract.c
> > % head: 1.4
> > % branch:
> > % locks: strict
> > % access list:
> > % symbolic names:
> > % RELENG_4: 1.4.0.2
> > ^^^^^^^^
> > % TAR_v1_13_25: 1.1.1.1
> > % FSF: 1.1.1
> > % keyword substitution: kv
> > % total revisions: 6; selected revisions: 6
> > % description:
> > % ...
> > % ----------------------------
> > % revision 1.3
> > % date: 2002/06/07 06:02:35; author: sobomax; state: Exp; lines: +1 -1
> > % Disabling automatic --same-owner option when running as uid 0 along with
> > % the --same-permissions was an overkill, so put it back. This is consistent
> > % with what our old tar did.
> > %
> > % Suggested by: dillon
> > % ----------------------------
> > % revision 1.2
> > % date: 2002/06/07 00:03:23; author: sobomax; state: Exp; lines: +4 -0
> > % IMO it was a quite ugly idea that if we are running as uid 0 then we can
> > % safely ignore current umask(2) and assume that permissions should be set
> > % right like in the archive. Not only it violates POLA, but introduces
> > ^^^^^^^^^^^^^
> > % huge potential security vulnerability, particularly for ports, where
> > % many popular archives come with 777 files and dirs.
> > % ----------------------------
> >
> > Actually, it is the change violates POLA, and breaks everything that
> > depends on the historical and still documented behaviour. (The man
> > page even says that (almost) all permissions are restored even in the
> > !root case (it says this indirectly by saying that all attributes are
> > restored if possible and not mentioning the umask or root). The info
> > page is better.)
> >
> > This bug showed up as breakage in mutt. mutt uses a setgid utility
> > named mutt_dotlock to lock /var/mail/*, so it fails to download mail
> > if mutt_dotlock's setgid bit is lost on extraction. It is probably
> > another bug that mutt_dotlock attempts to create a temporary file in
> > /var/mail instead of using flock().
> >
> > "Fixes":
> >
> > (1) Change pkg_add and thousands of user scripts to use tar -p. This
> > may reopen security holes closed by respecting the umask.
> >
> > %%%
> > Index: extract.c
> > ===================================================================
> > RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
> > retrieving revision 1.33
> > diff -u -2 -r1.33 extract.c
> > --- extract.c 11 May 2002 04:17:54 -0000 1.33
> > +++ extract.c 1 Aug 2002 10:26:10 -0000
> > @@ -33,5 +33,5 @@
> > #define PUSHOUT(todir) /* push out string */ \
> > if (where_count > (int)sizeof(STARTSTRING)-1) { \
> > - strcat(where_args, "|tar --unlink -xf - -C "); \
> > + strcat(where_args, "|tar --unlink -pxf - -C "); \
> > strcat(where_args, todir); \
> > if (system(where_args)) { \
> > %%%
> >
> > (2) Restore standard gnu tar behaviour by backing out extract.c revs 1.2-1.3.
> >
> > %%%
> > Index: extract.c
> > ===================================================================
> > RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
> > retrieving revision 1.4
> > diff -u -2 -r1.4 extract.c
> > --- extract.c 3 Jul 2002 12:44:31 -0000 1.4
> > +++ extract.c 1 Aug 2002 10:44:34 -0000
> > @@ -113,7 +113,5 @@
> > {
> > we_are_root = geteuid () == 0;
> > -#ifndef __FreeBSD__
> > same_permissions_option += we_are_root;
> > -#endif
> > same_owner_option += we_are_root;
> > xalloc_fail_func = extract_finish;
> > %%%
> >
> > Bruce
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3D495C93.C7198139>