Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 01 Aug 2002 18:56:22 +0300
From:      Maxim Sobolev <sobomax@FreeBSD.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        current@FreeBSD.org, obrien@FreeBSD.org
Subject:   Re: pkg_add broken by POLA breakage in tar
Message-ID:  <3D495A26.4627C170@FreeBSD.org>
References:  <20020801203312.V1911-100000@gamplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

-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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3D495A26.4627C170>