Skip site navigation (1)Skip section navigation (2)
Date:      24 Oct 1997 01:59:14 GMT
From:      nnd@itfs.nsk.su
To:        bugs@freebsd.org
Subject:   Re: bin/4829: ftpd does not check user's gid for groups entries in ftpchroot and ftpusers
Message-ID:  <62ovdi$jh6@news.itfs.nsk.su>
References:  <199710230319.JAA28671@uddias.diaspro.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Vasim Valejev <vasim@uddias.diaspro.com> wrote:

> >Number:         4829
> >Category:       bin
> >Synopsis:       ftpd does not check user's gid for groups entries
>                  in ftpchroot and ftpusers
	......

> >Environment:

> 	FreeBSD 2.2.2-RELEASE

> >Description:

>       ftpd does not chroot for users with group-id
>        from group's entry in ftpchroot
>      (will chroot only for usernames in /etc/ftpchroot and /etc/group)


	FreeBSD-current's ftpd still do the same - i.e. if
I have for example

@dialin

in /etc/ftpchroot file and make group-id for all my dial-in
users  'dialin' (and not explicitly list all them in 'dialin' group
in /etc/group) then ftpd will not 'chroot' them.

	So, the problem described in this PR is real, but
the fix proposed is

1) incorrect - there is pwuser->pw_gig clause without testing
   pwuser for non-NULL-ness (f.e. for user=anonymous);

2) unefficient - getpwnam in patch doubles sgetpwnam in
   function calling 'checkuser';


	Here is another patch (for 3.0 sources) which solves
the same problem:

==============================================================
--- ftpd.c.ORIG	Fri Oct 24 08:37:11 1997
+++ ftpd.c	Fri Oct 24 08:46:38 1997
@@ -234,7 +234,7 @@
 #endif
 static void	 ack __P((char *));
 static void	 myoob __P((int));
-static int	 checkuser __P((char *, char *));
+static int       checkuser __P((char *, char *, int));
 static FILE	*dataconn __P((char *, off_t, char *));
 static void	 dolog __P((struct sockaddr_in *));
 static char	*curdir __P((void));
@@ -777,14 +777,16 @@
 
 	guest = 0;
 	if (strcmp(name, "ftp") == 0 || strcmp(name, "anonymous") == 0) {
-		if (checkuser(_PATH_FTPUSERS, "ftp") ||
-		    checkuser(_PATH_FTPUSERS, "anonymous"))
-			reply(530, "User %s access denied.", name);
 #ifdef VIRTUAL_HOSTING
-		else if ((pw = sgetpwnam(thishost->anonuser)) != NULL) {
+		if ((pw = sgetpwnam(thishost->anonuser)) == NULL)
 #else
-		else if ((pw = sgetpwnam("ftp")) != NULL) {
+		if ((pw = sgetpwnam("ftp")) == NULL)
 #endif
+			reply(530, "User %s unknown.", name);
+		if (checkuser(_PATH_FTPUSERS, "ftp", pw->pw_gid) ||
+		    checkuser(_PATH_FTPUSERS, "anonymous", pw->pw_gid))
+			reply(530, "User %s access denied.", name);
+		else {
 			guest = 1;
 			askpasswd = 1;
 			reply(331,
@@ -809,7 +811,7 @@
 				break;
 		endusershell();
 
-		if (cp == NULL || checkuser(_PATH_FTPUSERS, name)) {
+		if (cp == NULL || checkuser(_PATH_FTPUSERS, name, pw->pw_gid)) {
 			reply(530, "User %s access denied.", name);
 			if (logging)
 				syslog(LOG_NOTICE,
@@ -840,9 +842,10 @@
  * Check if a user is in the file "fname"
  */
 static int
-checkuser(fname, name)
+checkuser(fname, name, gid)
 	char *fname;
 	char *name;
+	int gid;
 {
 	FILE *fd;
 	int found = 0;
@@ -863,6 +866,7 @@
 
 					if ((grp = getgrnam(line+1)) == NULL)
 						continue;
+					found = (gid == grp->gr_gid);
 					while (!found && grp->gr_mem[i])
 						found = strcmp(name,
 							grp->gr_mem[i++])



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?62ovdi$jh6>