Date: Mon, 18 Aug 1997 19:26:01 -0700 From: Cy Schubert <cy@cwsys.cwent.com> To: security-officer@freebsd.org Cc: freebsd-security@freebsd.org Subject: ftp(1) security hole, and suggested fixes Message-ID: <199708190226.TAA02995@cwsys.cwent.com>
next in thread | raw e-mail | index | archive | help
There's been some discussion on NetBSD's tech-security list about a security
hole in the ftp client. Enclosed is the original posting and a proposed
fix for the problem.
Does anyone have any thoughts on this?
Regards, Phone: (250)387-8437
Cy Schubert Fax: (250)387-5766
UNIX Support OV/VM: BCSC02(CSCHUBER)
ITSD BITNET: CSCHUBER@BCSC02.BITNET
Government of BC Internet: cschuber@uumail.gov.bc.ca
cschuber@bcsc02.gov.bc.ca
"Quit spooling around, JES do it."
------- Forwarded Message
Received: (from uucp@localhost) by passer.osg.gov.bc.ca (8.8.5/8.6.10) id HAA23925 for <cy@passer.osg.gov.bc.ca>; Sun, 17 Aug 1997 07:26:28 -0700 (PDT)
X-UIDL: 871832360.000
Resent-Message-Id: <199708171426.HAA23925@passer.osg.gov.bc.ca>
Received: from localhost(127.0.0.1), claiming to be "passer.osg.gov.bc.ca"
via SMTP by localhost, id smtpdAAAaaxBba; Sun Aug 17 07:26:22 1997
Received: (from uucp@localhost) by passer.osg.gov.bc.ca (8.8.5/8.6.10) id HAA24452 for <cschuber@passer.osg.gov.bc.ca>; Sun, 17 Aug 1997 07:26:18 -0700 (PDT)
Received: from orca.gov.bc.ca(142.32.102.25)
via SMTP by passer.osg.gov.bc.ca, id smtpdAAAaaxcja; Sun Aug 17 07:26:13 1997
Received: from homeworld.cygnus.com by orca.gov.bc.ca (5.4R3.10/200.1.1.4)
id AA22609; Sun, 17 Aug 1997 07:26:11 -0700
Received: (qmail 9384 invoked by uid 605); 17 Aug 1997 14:25:42 -0000
Received: (qmail 9378 invoked from network); 17 Aug 1997 14:25:37 -0000
Received: from goanna.cs.rmit.edu.au (lm@131.170.24.40)
by homeworld.cygnus.com with SMTP; 17 Aug 1997 14:25:37 -0000
Received: (from lm@localhost)
by goanna.cs.rmit.edu.au (8.8.6/8.8.6) id AAA15240
for tech-security@netbsd.org; Mon, 18 Aug 1997 00:25:29 +1000 (EST)
Message-Id: <199708171425.AAA15240@goanna.cs.rmit.edu.au>
Subject: ftp(1) security hole, and suggested fixes
To: tech-security@netbsd.org
Date: Mon, 18 Aug 1997 00:25:29 +1000 (EST)
From: Luke Mewburn <lm@rmit.edu.au>
Reply-To: Luke Mewburn <lm@rmit.edu.au>
X-Mailer: ELM [version 2.4 PL25]
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: tech-security-owner@netbsd.org
Precedence: list
Delivered-To: tech-security@NetBSD.ORG
Resent-To: cy@passer.osg.gov.bc.ca
Resent-Date: Sun, 17 Aug 1997 07:26:20 -0700
Resent-From: Cy Schubert - ITSD Open Systems Group <cschuber@passer.osg.gov.bc.ca>
Recently someone noted on BUGTRAQ that ftp(1) has two security
problems:
Problem:
a remote ftp server can create unwanted files by returning a list of
filenames to mget that aren't what the client asked for. Depending
upon the complexity of attack by the hostile server, it may be
rather difficult for a client to detect this in situ.
E.g, if ftp is run in ~/foo, and "mget *" returns a list of filenames
including "../.rhosts" with appropriate contents, then an unwary
user or one who has disabled prompting may find their account wide
open. Other attacks are possible.
Suggested fix:
check the returned filenames against the local glob rules, and
discard those that don't match (e.g, "../.forward" doesn't match
"foo*"). this could be configurable with an option, and default
to "do the check".
I haven't done this yet, as I'm awaiting feedback on the idea.
Problem:
it is possible to trick the client into executing arbitrary code
on the client's machine by returning a filename such as '|sh',
whose contents are the list of shell commands to execute.
Suggested fix:
modify recvrequest() to take an extra argument, which means
"ignore special names such as '-' and '|*'". use this flag
when calling recvrequest() from mget().
I've done this, and it appears to work.
Comments?
- --
Luke Mewburn, <lukem@netbsd.org>
(Message inbox:4)
Received: (from uucp@localhost) by passer.osg.gov.bc.ca (8.8.5/8.6.10) id LAA24720 for <cy@passer.osg.gov.bc.ca>; Sun, 17 Aug 1997 11:43:48 -0700 (PDT)
X-UIDL: 871951866.002
Resent-Message-Id: <199708171843.LAA24720@passer.osg.gov.bc.ca>
Received: from localhost(127.0.0.1), claiming to be "passer.osg.gov.bc.ca"
via SMTP by localhost, id smtpdAAAaaykra; Sun Aug 17 11:43:39 1997
Received: (from uucp@localhost) by passer.osg.gov.bc.ca (8.8.5/8.6.10) id LAA24658 for <cschuber@passer.osg.gov.bc.ca>; Sun, 17 Aug 1997 11:43:38 -0700 (PDT)
Received: from orca.gov.bc.ca(142.32.102.25)
via SMTP by passer.osg.gov.bc.ca, id smtpdAAAaayqBa; Sun Aug 17 11:43:37 1997
Received: from homeworld.cygnus.com by orca.gov.bc.ca (5.4R3.10/200.1.1.4)
id AA22681; Sun, 17 Aug 1997 11:43:34 -0700
Received: (qmail 20625 invoked by uid 605); 17 Aug 1997 18:42:50 -0000
Received: (qmail 20619 invoked from network); 17 Aug 1997 18:42:44 -0000
Received: from burgundy.eecs.harvard.edu (dholland@140.247.60.165)
by homeworld.cygnus.com with SMTP; 17 Aug 1997 18:42:44 -0000
Received: (from dholland@localhost)
by burgundy.eecs.harvard.edu (8.8.5/8.8.5) id OAA03099;
Sun, 17 Aug 1997 14:42:28 -0400 (EDT)
From: David Holland <dholland@eecs.harvard.edu>
Message-Id: <199708171842.OAA03099@burgundy.eecs.harvard.edu>
Subject: Re: ftp(1) security hole, and suggested fixes
To: lm@rmit.edu.au
Date: Sun, 17 Aug 1997 14:42:28 -0400 (EDT)
Cc: tech-security@netbsd.org
In-Reply-To: <199708171425.AAA15240@goanna.cs.rmit.edu.au> from "Luke Mewburn" at Aug 18, 97 00:25:29 am
X-Mailer: ELM [version 2.4 PL25]
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: tech-security-owner@netbsd.org
Precedence: list
Delivered-To: tech-security@NetBSD.ORG
Resent-To: cy@passer.osg.gov.bc.ca
Resent-Date: Sun, 17 Aug 1997 11:43:39 -0700
Resent-From: Cy Schubert - ITSD Open Systems Group <cschuber@passer.osg.gov.bc.ca>
> Recently someone noted on BUGTRAQ that ftp(1) has two security
> problems:
> [...]
>
> Problem:
> it is possible to trick the client into executing arbitrary code
> on the client's machine by returning a filename such as '|sh',
> whose contents are the list of shell commands to execute.
>
> Suggested fix:
> modify recvrequest() to take an extra argument, which means
> "ignore special names such as '-' and '|*'". use this flag
> when calling recvrequest() from mget().
> I've done this, and it appears to work.
On second thought I don't think this is ideal: you also, if possible,
want to protect against someone seeing a file called '|sh' on an ftp
server and trying to get it. If you do "get |sh", at present, the
contents of the file on the server will be piped to sh since get
copies its argv[1] to argv[2] if the user doesn't supply one.
So I propose instead prepending "./" to any automatically generated
local name, both in get and mget, that begins with '|' or is '-'.
Also, names beginning with '/' should have a '.' prepended to them --
otherwise the server can send "/root/.rhosts" or
"/home/victim/.rhosts" or whatever.
Additionally, everything that mget generates should have ".." path
elements filtered out. I've left off this processing for get, on the
grounds that someone who does "get ../foo" should expect the file to
appear in the parent directory. Maybe this should be changed to be
more consistent.
My fix does reasonably unhappy things if you do "mget ../*".
Suggestions for improvements are welcome.
The following patch fixes the Linux ftp client, and should be pretty
readily adaptable to the NetBSD one.
--- cmds.c 1997/06/13 07:25:11 1.20
+++ cmds.c 1997/08/17 18:19:47
@@ -34,9 +34,9 @@
/*
* from: @(#)cmds.c 5.26 (Berkeley) 3/5/91
*/
char cmds_rcsid[] =
- "$Id: cmds.c,v 1.20 1997/06/13 07:25:11 dholland Exp $";
+ "$Id: cmds.c,v 1.23 1997/08/17 18:18:30 dholland Exp $";
/*
* FTP User Program -- Command Routines.
*/
@@ -90,8 +90,67 @@
static void quote1(const char *initial, int argc, char **argv);
/*
+ * pipeprotect: protect against "special" local filenames by prepending
+ * "./". Special local filenames are "-" and "|..." AND "/...".
+ */
+static char *pipeprotect(char *name)
+{
+ char *nu;
+ if (strcmp(name, "-") && *name!='|' && *name!='/') {
+ return name;
+ }
+
+ /* We're going to leak this memory. XXX. */
+ nu = malloc(strlen(name)+3);
+ if (nu==NULL) {
+ perror("malloc");
+ code = -1;
+ return NULL;
+ }
+ strcpy(nu, ".");
+ if (*name != '/') strcat(nu, "/");
+ strcat(nu, name);
+ return nu;
+}
+
+/*
+ * Look for embedded ".." in a pathname and change it to "!!", printing
+ * a warning.
+ */
+static char *pathprotect(char *name)
+{
+ int gotdots=0, i, len;
+
+ /* Convert null terminator to trailing / to catch a trailing ".." */
+ len = strlen(name)+1;
+ name[len-1] = '/';
+
+ /*
+ * State machine loop. gotdots is < 0 if not looking at dots,
+ * 0 if we just saw a / and thus might start getting dots,
+ * and the count of dots seen so far if we have seen some.
+ */
+ for (i=0; i<len; i++) {
+ if (name[i]=='.' && gotdots>=0) gotdots++;
+ else if (name[i]=='/' && gotdots<0) gotdots=0;
+ else if (name[i]=='/' && gotdots==2) {
+ printf("Warning: embedded .. in %.*s (changing to !!)\n",
+ len-1, name);
+ name[i-1] = '!';
+ name[i-2] = '!';
+ gotdots = 0;
+ }
+ else if (name[i]=='/') gotdots = 0;
+ else gotdots = -1;
+ }
+ name[len-1] = 0;
+ return name;
+}
+
+
+/*
* `Another' gets another argument, and stores the new argc and argv.
* It reverts to the top level (via main.c's intr()) on EOF/error.
*
* Returns false if no new arguments have been added.
@@ -594,9 +653,17 @@
char *oldargv1, *oldargv2;
if (argc == 2) {
argc++;
- argv[2] = argv[1];
+ /*
+ * Protect the user from accidentally retrieving special
+ * local names.
+ */
+ argv[2] = pipeprotect(argv[1]);
+ if (!argv[2]) {
+ code = -1;
+ return 0;
+ }
loc++;
}
if (argc < 2 && !another(&argc, &argv, "remote-file"))
goto usage;
@@ -768,10 +835,21 @@
}
if (mapflag) {
tp = domap(tp);
}
- recvrequest("RETR", tp, cp, "w",
- tp != cp || !interactive);
+ /* Reject embedded ".." */
+ tp = pathprotect(tp);
+
+ /* Prepend ./ to "-" or "!*" or leading "/" */
+ tp = pipeprotect(tp);
+ if (tp == NULL) {
+ /* hmm... how best to handle this? */
+ mflag = 0;
+ }
+ else {
+ recvrequest("RETR", tp, cp, "w",
+ tp != cp || !interactive);
+ }
if (!mflag && fromatty) {
ointer = interactive;
interactive = 1;
if (confirm("Continue with","mget")) {
--
- David A. Holland | VINO project home page:
dholland@eecs.harvard.edu | http://www.eecs.harvard.edu/vino
------- End of Forwarded Message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199708190226.TAA02995>