Skip Menu |
 

Download (untitled) / with headers
text/plain 3.6KiB
From krb5-bugs-incoming-bounces@PCH.MIT.EDU Tue Jul 17 15:57:07 2007
Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (8.12.9) with ESMTP
id l6HJv7HW023651; Tue, 17 Jul 2007 15:57:07 -0400 (EDT)
Received: from pch.mit.edu (pch.mit.edu [127.0.0.1])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id l6HJv2sx022826;
Tue, 17 Jul 2007 15:57:02 -0400
Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU
[18.7.7.76])
by pch.mit.edu (8.13.6/8.12.8) with ESMTP id l6HJUHVV015891
for <krb5-bugs-incoming@PCH.mit.edu>; Tue, 17 Jul 2007 15:30:17 -0400
Received: from mit.edu (W92-130-BARRACUDA-2.MIT.EDU [18.7.21.223])
by fort-point-station.mit.edu (8.13.6/8.9.2) with ESMTP id
l6HJUBQN008050
for <krb5-bugs@mit.edu>; Tue, 17 Jul 2007 15:30:12 -0400 (EDT)
Received: from mx1.redhat.com (mx1.redhat.com [66.187.233.31])
by mit.edu (Spam Firewall) with ESMTP id 345D13A5D0C
for <krb5-bugs@mit.edu>; Tue, 17 Jul 2007 15:30:11 -0400 (EDT)
Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com
[172.16.52.254])
by mx1.redhat.com (8.13.1/8.13.1) with ESMTP id l6HJUAsb015081
for <krb5-bugs@mit.edu>; Tue, 17 Jul 2007 15:30:10 -0400
Received: from rapier.boston.redhat.com (rapier.boston.redhat.com
[172.16.80.53])
by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id l6HJU9FO004034
for <krb5-bugs@mit.edu>; Tue, 17 Jul 2007 15:30:10 -0400
Received: from rapier.boston.redhat.com (localhost.localdomain [127.0.0.1])
by rapier.boston.redhat.com (8.14.1/8.14.0) with ESMTP id
l6HJU9Wi009865
for <krb5-bugs@mit.edu>; Tue, 17 Jul 2007 15:30:09 -0400
Received: (from nalin@localhost)
by rapier.boston.redhat.com (8.14.1/8.14.1/Submit) id l6HJU93Q009864;
Tue, 17 Jul 2007 15:30:09 -0400
Date: Tue, 17 Jul 2007 15:30:09 -0400
Message-Id: <200707171930.l6HJU93Q009864@rapier.boston.redhat.com>
To: krb5-bugs@mit.edu
Subject: ftp: error reading remote file on passive get results in leaked
descriptor
From: nalin@redhat.com
X-send-pr-version: 3.99
X-Spam-Score: 0.55
X-Spam-Flag: NO
X-Scanned-By: MIMEDefang 2.42
X-Mailman-Approved-At: Tue, 17 Jul 2007 15:57:01 -0400
X-BeenThere: krb5-bugs-incoming@mailman.mit.edu
X-Mailman-Version: 2.1.6
Precedence: list
Reply-To: nalin@redhat.com
Sender: krb5-bugs-incoming-bounces@PCH.MIT.EDU
Errors-To: krb5-bugs-incoming-bounces@PCH.MIT.EDU


Show quoted text
>Submitter-Id: net
>Originator:
>Organization:
>Confidential: no
>Synopsis: ftp: error reading remote file on passive get results in leaked descriptor
>Severity: non-critical
>Priority: low
>Category: krb5-appl
>Class: sw-bug
>Release: 1.6.1
>Environment:

System: Linux rapier.boston.redhat.com 2.6.21-1.3230.fc8 #1 SMP Wed Jun 20 15:59:23 EDT 2007 x86_64 x86_64 x86_64 GNU/Linux
Architecture: x86_64

Show quoted text
>Description:
In passive mode, if a file "get" fails because access to the file
on the server was denied (say, because we tried to read /etc/shadow),
the client leaks a descriptor.
Show quoted text
>How-To-Repeat:
Please a mode 600 file on the FTP server. Connect, go into passive
mode, and attempt to retrieve the file. Repeat. If you're tracking
in-use descriptors, you should see the use count go up each time.
Show quoted text
>Fix:
Close the data descriptor if we're following this path, like the
client does in other parts of the same file:

Index: src/appl/gssftp/ftp/ftp.c
===================================================================
--- src/appl/gssftp/ftp/ftp.c (revision 19714)
+++ src/appl/gssftp/ftp/ftp.c (working copy)
@@ -1450,6 +1450,8 @@
int a1,a2,a3,a4,p1,p2;

if (passivemode) {
+ if (data != INVALID_SOCKET)
+ (void) closesocket(data);
data = socket(AF_INET, SOCK_STREAM, 0);
if (data == INVALID_SOCKET) {
PERROR_SOCKET("ftp: socket");
Date: Mon, 17 Sep 2007 18:17:23 -0400
From: Nalin Dahyabhai <nalin@redhat.com>
To: RT <rt@krbdev.mit.edu>
Subject: Re: [krbdev.mit.edu #5597] ftp: error reading remote file on passive get results in leaked descriptor
RT-Send-Cc:
Download (untitled) / with headers
text/plain 2.3KiB
Hmm, my previously-proposed patch turned out to miss the case where the
descriptor is open and is the temporarly list of files to be retrieved
during an "mget". This revised version sets the variable which holds
that descriptor to INVALID_SOCKET when that happens. This requires
storing a copy of the descriptor elsewhere in the block where we pass it
to FDOPEN_SOCKET twice.

On non-Win32 boxes, it's also passed to dup() first, so that the two
resulting FILE structures don't share the same underlying descriptor.
I'm not sure if that's _really_ necessary, but it felt more correct.

Index: src/appl/gssftp/ftp/ftp_var.h
===================================================================
--- src/appl/gssftp/ftp/ftp_var.h (revision 19936)
+++ src/appl/gssftp/ftp/ftp_var.h (working copy)
@@ -48,7 +48,8 @@
#define PERROR_SOCKET(str) do { errno = SOCKET_ERRNO; perror(str); } while(0)
#else
#define FCLOSE_SOCKET(f) fclose(f)
-#define FDOPEN_SOCKET(s, mode) fdopen(s, mode)
+FILE* fdopen_socket(int *s, char* mode);
+#define FDOPEN_SOCKET(s, mode) fdopen_socket(&s, mode)
#define SOCKETNO(fd) (fd)
#define PERROR_SOCKET(str) perror(str)
#endif
Index: src/appl/gssftp/ftp/ftp.c
===================================================================
--- src/appl/gssftp/ftp/ftp.c (revision 19936)
+++ src/appl/gssftp/ftp/ftp.c (working copy)
@@ -196,7 +196,7 @@
hookup(char* host, int port)
{
register struct hostent *hp = 0;
- int s;
+ int s, t;
socklen_t len;
#ifdef IP_TOS
#ifdef IPTOS_LOWDELAY
@@ -274,8 +274,13 @@
}
#endif
#endif
+#ifndef _WIN32
+ t = dup(s);
+#else
+ t = s;
+#endif
cin = FDOPEN_SOCKET(s, "r");
- cout = FDOPEN_SOCKET(s, "w");
+ cout = FDOPEN_SOCKET(t, "w");
if (cin == NULL || cout == NULL) {
fprintf(stderr, "ftp: fdopen failed.\n");
if (cin) {
@@ -1450,6 +1455,8 @@
int a1,a2,a3,a4,p1,p2;

if (passivemode) {
+ if (data != INVALID_SOCKET)
+ (void) closesocket(data);
data = socket(AF_INET, SOCK_STREAM, 0);
if (data == INVALID_SOCKET) {
PERROR_SOCKET("ftp: socket");
@@ -2365,4 +2372,16 @@

return f;
}
+#else
+/* Non-Win32 case takes the address of the variable so that we can "take
+ * ownership" of the descriptor number. */
+FILE* fdopen_socket(int *s, char* mode)
+{
+ FILE *fp;
+ fp = fdopen(*s, mode);
+ if (fp) {
+ *s = INVALID_SOCKET;
+ }
+ return fp;
+}
#endif /* _WIN32 */