From owner-freebsd-bugs@FreeBSD.ORG Mon Feb 27 21:20:30 2006 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1E04316A420 for ; Mon, 27 Feb 2006 21:20:30 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2787B43D4C for ; Mon, 27 Feb 2006 21:20:13 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k1RLK3mj099796 for ; Mon, 27 Feb 2006 21:20:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k1RLK37v099795; Mon, 27 Feb 2006 21:20:03 GMT (envelope-from gnats) Resent-Date: Mon, 27 Feb 2006 21:20:03 GMT Resent-Message-Id: <200602272120.k1RLK37v099795@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Jason Heiss Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9DDBD16A420 for ; Mon, 27 Feb 2006 21:17:16 +0000 (GMT) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [216.136.204.117]) by mx1.FreeBSD.org (Postfix) with ESMTP id E1BC143D7B for ; Mon, 27 Feb 2006 21:17:07 +0000 (GMT) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.13.1/8.13.1) with ESMTP id k1RLH7Wb020585 for ; Mon, 27 Feb 2006 21:17:07 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.13.1/8.13.1/Submit) id k1RLH7XQ020584; Mon, 27 Feb 2006 21:17:07 GMT (envelope-from nobody) Message-Id: <200602272117.k1RLH7XQ020584@www.freebsd.org> Date: Mon, 27 Feb 2006 21:17:07 GMT From: Jason Heiss To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-2.3 Cc: Subject: bin/93915: pkg_add behaves improperly when unpacking over symlinks X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Feb 2006 21:20:30 -0000 >Number: 93915 >Category: bin >Synopsis: pkg_add behaves improperly when unpacking over symlinks >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Feb 27 21:20:03 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Jason Heiss >Release: 6.0 >Organization: Yahoo! >Environment: >Description: The behavior of pkg_add when unpacking over symlinks changed for the worse between 4.x and 6.x (I believe specifically it changed in 5.3 when bsdtar was made the default tar). Assume you have a package containing only the following file (i.e. the package does not specifically list /usr/local as a component of the package): /usr/local/bin/foo And assume one of the following two directory structures exist prior to installing that package: /usr/local -> /opt/local /opt/local/bin/ /usr/local/bin/foo -> /usr/local/bin/bar When unpacking over the first directory structure I would expect foo to end up in /opt/local/bin/foo. Because the package does not specifically say that /usr/local should be a directory I would expect pkg_add to traverse the symlink. (I can imagine arguments against this behavior, which I'll discuss in more detail later.) When unpacking over the second directory structure I would expect the existing /usr/local/bin/foo symlink to be removed, and the /usr/local/bin/foo from the package to be installed. Because /usr/local/bin/foo is specifically listed as a component of the package I would expect any existing file in that location to be removed and replaced by the file in the package. Both of these are the behaviors you see in older versions of FreeBSD. However, in current versions when unpacking over the first directory structure the /usr/local symlink is removed and the directory structure within the package is created. That definitely violates the "principal of least astonishment", and also is a unexpected/undocumented change in behavior. I believe it is sufficiently unexpected as to qualify as a bug. I believe the reason for this change in behavior is the replacement of the underlying tar command in the 5.3 release, and more specifically the old GNU tar and bsdtar handling the --unlink option differently. In October 1998 the --unlink option was added to the options passed to tar by pkg_add when unpacking files in their final destination (see the PUSHOUT macro in src/usr.sbin/pkg_install/add/extract.c). The CVS commit message gives no reason for the change, and I'm not clear what the --unlink option does in the old GNU tar (it seems to have no effect in either of the example situations I gave earlier). With bsdtar the --unlink option results in the behavior I am reporting, where the /usr/local symlink is removed. This appears to be the documented and intended behavior of the --unlink option in bsdtar, so I don't consider that a bug in bsdtar. However, I don't believe pkg_add should be exhibiting this behavior, which implies that pkg_add should not specify the --unlink option when tar==bsdtar. The question then is can you simply remove the --unlink flag (reverse the change of October 1998) and have acceptable behavior? I believe the answer is no, but I can imagine debate on this point (as mentioned earlier). The two reasonable behaviors would be: 1) Silently traverse any symlinks in the path. This was the behavior of older versions of FreeBSD, and I believe this to be the most reasonable behavior for pkg_add. 2) Error out if unpacking any files requires traversing a symlink. This is the behavior of bsdtar in the absence of the --unlink flag. When bsdtar is confronted with the first example directory structure and you have not specified the --unlink flag it will simply refuse to follow the symlink and unpack the file. (See the security_problem function in src/usr.bin/tar/read.c) This is behavior 2 above, and is the easiest to achieve by simply removing the --unlink flag in extract.c. In order to achieve behavior 1 we have to suppress the symlink traversal detection of the security_problem function within bsdtar. The only way to suppress that behavior is through the -P flag, which has other unwanted side effects. I think the only way to achieve this behavior would be to add a new option to bsdtar which is a subset of the -P behavior, just suppressing the symlink detection. In summary, I recommend that the --unlink flag to tar be immediately removed from the PUSHOUT macro of extract.c within pkg_add. That will removed a very unexpected and unacceptable behavior. A possible remaining improvement would be to replace that with a flag to tar which disables symlink traversal detection, such a flag does not currently exist in bsdtar. >How-To-Repeat: >Fix: This should be applied to any release where tar==bsdtar: Index: src/usr.sbin/pkg_install/add/extract.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v retrieving revision 1.44 diff -u -r1.44 extract.c --- src/usr.sbin/pkg_install/add/extract.c 7 Jan 2006 22:10:57 -0000 1.44 +++ src/usr.sbin/pkg_install/add/extract.c 27 Feb 2006 21:15:11 -0000 @@ -34,7 +34,7 @@ #define PUSHOUT(todir) /* push out string */ \ if (where_count > (int)sizeof(STARTSTRING)-1) { \ - strcat(where_args, "|/usr/bin/tar --unlink -xpf - -C "); \ + strcat(where_args, "|/usr/bin/tar -xpf - -C "); \ strcat(where_args, todir); \ if (system(where_args)) { \ cleanup(0); \ >Release-Note: >Audit-Trail: >Unformatted: