Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 1 Aug 2002 21:41:24 +1000 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        sobomax@freebsd.org
Cc:        current@freebsd.org, <obrien@freebsd.org>
Subject:   pkg_add broken by POLA breakage in tar
Message-ID:  <20020801203312.V1911-100000@gamplex.bde.org>

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

% 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?20020801203312.V1911-100000>