From owner-cvs-bin Mon Jun 8 20:41:45 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id UAA16673 for cvs-bin-outgoing; Mon, 8 Jun 1998 20:41:45 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id UAA16303; Mon, 8 Jun 1998 20:40:10 -0700 (PDT) (envelope-from imp@FreeBSD.org) From: Warner Losh Received: (from imp@localhost) by freefall.freebsd.org (8.8.8/8.8.5) id UAA16457; Mon, 8 Jun 1998 20:38:45 -0700 (PDT) Date: Mon, 8 Jun 1998 20:38:45 -0700 (PDT) Message-Id: <199806090338.UAA16457@freefall.freebsd.org> To: cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG Subject: cvs commit: src/bin/cp utils.c src/bin/csh dir.c src/bin/pax ftree.c Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk imp 1998/06/08 20:38:44 PDT Modified files: bin/cp utils.c bin/csh dir.c bin/pax ftree.c Log: Make sure we pass the length - 1 to readlink, since it adds its own NUL at the end of the path. Inspired by: OpenBSD's changes in this area by theo de raadt Revision Changes Path 1.19 +2 -2 src/bin/cp/utils.c 1.8 +3 -5 src/bin/csh/dir.c 1.11 +3 -3 src/bin/pax/ftree.c From owner-cvs-bin Mon Jun 8 20:41:58 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id UAA16704 for cvs-bin-outgoing; Mon, 8 Jun 1998 20:41:58 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id UAA16529; Mon, 8 Jun 1998 20:41:07 -0700 (PDT) (envelope-from imp@FreeBSD.org) From: Warner Losh Received: (from imp@localhost) by freefall.freebsd.org (8.8.8/8.8.5) id UAA16484; Mon, 8 Jun 1998 20:39:40 -0700 (PDT) Date: Mon, 8 Jun 1998 20:39:40 -0700 (PDT) Message-Id: <199806090339.UAA16484@freefall.freebsd.org> To: cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG Subject: cvs commit: src/bin/mv mv.c Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk imp 1998/06/08 20:39:40 PDT Modified files: bin/mv mv.c Log: Make sure we don't overflow the path buffer. Exit if we do. Obtained from or inspired by: A similar change in OpenBSD by theo Revision Changes Path 1.20 +4 -2 src/bin/mv/mv.c From owner-cvs-bin Mon Jun 8 22:37:56 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id WAA05508 for cvs-bin-outgoing; Mon, 8 Jun 1998 22:37:56 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.15.68.22]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id WAA05417; Mon, 8 Jun 1998 22:37:18 -0700 (PDT) (envelope-from bde@godzilla.zeta.org.au) Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.7/8.8.7) id PAA01402; Tue, 9 Jun 1998 15:37:16 +1000 Date: Tue, 9 Jun 1998 15:37:16 +1000 From: Bruce Evans Message-Id: <199806090537.PAA01402@godzilla.zeta.org.au> To: cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG, cvs-committers@FreeBSD.ORG, imp@FreeBSD.ORG Subject: Re: cvs commit: src/bin/cp utils.c src/bin/csh dir.c src/bin/pax ftree.c Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk >imp 1998/06/08 20:38:44 PDT > > Modified files: > bin/cp utils.c > bin/csh dir.c > bin/pax ftree.c > Log: > Make sure we pass the length - 1 to readlink, since it adds its own > NUL at the end of the path. Actually, readlink() never NUL-terminates, but some buggy applications add their own NUL at the end of the path. Most applications use a buffer of size PATH_MAX, so it's not clear if there are any problems in practice. Does nfs_readlink() actually work for foreign links of size PATH_MAX? Bruce From owner-cvs-bin Mon Jun 8 22:46:10 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id WAA06877 for cvs-bin-outgoing; Mon, 8 Jun 1998 22:46:10 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from rover.village.org (rover.village.org [204.144.255.49]) by hub.freebsd.org (8.8.8/8.8.8) with SMTP id WAA06721; Mon, 8 Jun 1998 22:45:43 -0700 (PDT) (envelope-from imp@village.org) Received: from harmony [10.0.0.6] by rover.village.org with esmtp (Exim 1.71 #1) id 0yjHE5-0004oA-00; Mon, 8 Jun 1998 23:45:41 -0600 Received: from harmony.village.org (localhost [127.0.0.1]) by harmony.village.org (8.8.8/8.8.3) with ESMTP id XAA05689; Mon, 8 Jun 1998 23:46:01 -0600 (MDT) Message-Id: <199806090546.XAA05689@harmony.village.org> To: Bruce Evans Subject: Re: cvs commit: src/bin/cp utils.c src/bin/csh dir.c src/bin/pax ftree.c Cc: cvs-all@freebsd.org, cvs-bin@freebsd.org, cvs-committers@freebsd.org In-reply-to: Your message of "Tue, 09 Jun 1998 15:37:16 +1000." <199806090537.PAA01402@godzilla.zeta.org.au> References: <199806090537.PAA01402@godzilla.zeta.org.au> Date: Mon, 08 Jun 1998 23:46:01 -0600 From: Warner Losh Sender: owner-cvs-bin@freebsd.org X-Loop: FreeBSD.org Precedence: bulk In message <199806090537.PAA01402@godzilla.zeta.org.au> Bruce Evans writes: : Actually, readlink() never NUL-terminates, but some buggy applications : add their own NUL at the end of the path. Most applications use a buffer : of size PATH_MAX, so it's not clear if there are any problems in practice. This is true. The size returned from readlink is used to NUL terminate the string in applications. If we used the length returned when terminating the string, then we could have a one byte overflow. This patch fixes that. Also, most of the code that I recall seeing uses MAXPATHLEN, but that is defined to be PATH_MAX on FreeBSD, so you are correct. The man page states that when the length of the name exceeds PATH_MAX, it will return an error, but is silent on what happens if the path length is exactly equal to PATH_MAX. You are right that this likely isn't a problem or an exploitable hole, but a little prevention doesn't hurt, no? : Does nfs_readlink() actually work for foreign links of size PATH_MAX? I have no clue one way or the other about this. Warner From owner-cvs-bin Mon Jun 8 23:33:39 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id XAA12251 for cvs-bin-outgoing; Mon, 8 Jun 1998 23:33:39 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.15.68.22]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id XAA12197; Mon, 8 Jun 1998 23:33:11 -0700 (PDT) (envelope-from bde@godzilla.zeta.org.au) Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.7/8.8.7) id QAA05878; Tue, 9 Jun 1998 16:33:10 +1000 Date: Tue, 9 Jun 1998 16:33:10 +1000 From: Bruce Evans Message-Id: <199806090633.QAA05878@godzilla.zeta.org.au> To: bde@zeta.org.au, imp@village.org Subject: Re: cvs commit: src/bin/cp utils.c src/bin/csh dir.c src/bin/pax ftree.c Cc: cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG, cvs-committers@FreeBSD.ORG Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk >: Actually, readlink() never NUL-terminates, but some buggy applications >: add their own NUL at the end of the path. Most applications use a buffer >: of size PATH_MAX, so it's not clear if there are any problems in practice. I should have said that all applications (except very buggy ones :-) add the NUL, and some buggy ones add it after the end of their buffer. >This patch fixes that. Also, most of the code that I recall seeing >uses MAXPATHLEN, but that is defined to be PATH_MAX on FreeBSD, so you >are correct. The man page states that when the length of the name >exceeds PATH_MAX, it will return an error, but is silent on what >happens if the path length is exactly equal to PATH_MAX. Most of the code is crufty. PATH_MAX is POSIX standard, and POSIX has been Standard for almost 10 years now. It's fairly clear that MAXPATHLEN includes space for the terminating NUL (see intro.2). Most of man pages are too localised to BSD and give the limit as 1023 (PATH_MAX - 1). However, PATH_MAX seems to be irrelevant for path returned by readlink(). I think the kernel should return any name it can fit, and longer names can be generated on foreign systems. >You are right that this likely isn't a problem or an exploitable hole, >but a little prevention doesn't hurt, no? It hurts to complicate the code for no reason. Bruce From owner-cvs-bin Tue Jun 9 06:44:06 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id GAA23877 for cvs-bin-outgoing; Tue, 9 Jun 1998 06:44:06 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id GAA23674; Tue, 9 Jun 1998 06:42:57 -0700 (PDT) (envelope-from dt@FreeBSD.org) From: Dmitrij Tejblum Received: (from dt@localhost) by freefall.freebsd.org (8.8.8/8.8.5) id GAA00926; Tue, 9 Jun 1998 06:42:55 -0700 (PDT) Date: Tue, 9 Jun 1998 06:42:55 -0700 (PDT) Message-Id: <199806091342.GAA00926@freefall.freebsd.org> To: cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG Subject: cvs commit: src/bin/cp cp.c Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk dt 1998/06/09 06:42:54 PDT Modified files: bin/cp cp.c Log: Print correct error message if we copy an unreadable directory. Revision Changes Path 1.17 +2 -1 src/bin/cp/cp.c From owner-cvs-bin Tue Jun 9 23:30:01 1998 Return-Path: Received: (from daemon@localhost) by hub.freebsd.org (8.8.8/8.8.8) id XAA05321 for cvs-bin-outgoing; Tue, 9 Jun 1998 23:30:01 -0700 (PDT) (envelope-from owner-cvs-bin) Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id XAA05229; Tue, 9 Jun 1998 23:29:31 -0700 (PDT) (envelope-from peter@FreeBSD.org) From: Peter Wemm Received: (from peter@localhost) by freefall.freebsd.org (8.8.8/8.8.5) id XAA05606; Tue, 9 Jun 1998 23:29:25 -0700 (PDT) Date: Tue, 9 Jun 1998 23:29:25 -0700 (PDT) Message-Id: <199806100629.XAA05606@freefall.freebsd.org> To: cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG, cvs-bin@FreeBSD.ORG Subject: cvs commit: src/bin/cp utils.c Sender: owner-cvs-bin@FreeBSD.ORG X-Loop: FreeBSD.org Precedence: bulk peter 1998/06/09 23:29:25 PDT Modified files: bin/cp utils.c Log: Don't attempt to change owner/mode/flags that don't need to changed. This should calm down attempts to `cp -p' to a nfs mount or some other filesystem that doesn't accept flags or all combinations of flags. It will warn if it fails to change flags though. Revision Changes Path 1.20 +29 -15 src/bin/cp/utils.c