[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-inetutils] [PATCH] rexecd possible security problem
From: |
Giuseppe Scrivano |
Subject: |
Re: [bug-inetutils] [PATCH] rexecd possible security problem |
Date: |
Fri, 10 Jul 2009 11:45:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.94 (gnu/linux) |
"Alfred M. Szmidt" <address@hidden> writes:
> Please use error over fprintf/exit. Though I see that rexecd
> redefines error to something perverse... Would you like to fix that?
I used fprintf/exit just to be coherent with the rest of rexecd.
I amended my previous patch and merged the second one.
Cheers,
Giuseppe
>From dc1585b806d360ce684509ec62d8d7eb4b065024 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Thu, 9 Jul 2009 11:35:43 +0200
Subject: [PATCH] rexecd: check for errors of setegid/setuid/setgid/initgroups
2009-07-10 Giuseppe Scrivano <address@hidden>
* rexecd/rexecd.c (doit): Add return value check after
use setegid/setuid/setgid/initgroups and define its
prototype.
* rexecd/rexecd.c (error): Use the <error.h> version
in place of it.
---
rexecd/rexecd.c | 84 +++++++++++++++++++-----------------------------------
1 files changed, 30 insertions(+), 54 deletions(-)
diff --git a/rexecd/rexecd.c b/rexecd/rexecd.c
index be8511b..5832382 100644
--- a/rexecd/rexecd.c
+++ b/rexecd/rexecd.c
@@ -27,7 +27,7 @@
* SUCH DAMAGE.
*/
-/* Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008
+/* Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
Free Software Foundation, Inc.
This file is part of GNU Inetutils.
@@ -71,6 +71,7 @@
#include <netinet/in.h>
+#include <error.h>
#include <errno.h>
#include <netdb.h>
#include <pwd.h>
@@ -97,8 +98,8 @@
#endif
#include <progname.h>
-void error (const char *fmt, ...);
void usage (void);
+int doit (int, struct sockaddr_in *);
static const char *short_options = "hV";
static struct option long_options[] = {
@@ -141,10 +142,8 @@ main (int argc, char **argv)
fromlen = sizeof (from);
if (getpeername (sockfd, (struct sockaddr *) &from, &fromlen) < 0)
- {
- fprintf (stderr, "rexecd: getpeername: %s\n", strerror (errno));
- exit (1);
- }
+ error (EXIT_FAILURE, errno, "cannot retrieve peer address");
+
doit (sockfd, &from);
exit (0);
}
@@ -243,20 +242,14 @@ doit (int f, struct sockaddr_in *fromp)
setpwent ();
pwd = getpwnam (user);
if (pwd == NULL)
- {
- error ("Login incorrect.\n");
- exit (1);
- }
+ error (EXIT_FAILURE, 0, "login incorrect");
endpwent ();
pw_password = get_user_password (pwd);
if (*pw_password != '\0')
{
namep = crypt (pass, pw_password);
if (strcmp (namep, pw_password))
- {
- error ("Password incorrect.\n");
- exit (1);
- }
+ error (EXIT_FAILURE, 0, "password incorrect");
}
write (STDERR_FILENO, "\0", 1);
if (port)
@@ -264,10 +257,8 @@ doit (int f, struct sockaddr_in *fromp)
pipe (pv);
pid = fork ();
if (pid == -1)
- {
- error ("Try again.\n");
- exit (1);
- }
+ error (EXIT_FAILURE, errno, "cannot fork the process");
+
if (pid)
{
close (STDIN_FILENO);
@@ -320,17 +311,24 @@ doit (int f, struct sockaddr_in *fromp)
pwd->pw_shell = PATH_BSHELL;
if (f > 2)
close (f);
- setegid ((gid_t) pwd->pw_gid);
- setgid ((gid_t) pwd->pw_gid);
+
+ if (setegid ((gid_t) pwd->pw_gid) < 0)
+ error (EXIT_FAILURE, errno, "failed to set additional groups");
+
+ if (setgid ((gid_t) pwd->pw_gid) < 0)
+ error (EXIT_FAILURE, errno, "failed to set group-ID");
+
#ifdef HAVE_INITGROUPS
- initgroups (pwd->pw_name, pwd->pw_gid);
+ if (initgroups (pwd->pw_name, pwd->pw_gid) < 0)
+ error (EXIT_FAILURE, errno, "failed to initialize the supplementary group "
+ "access list");
#endif
- setuid ((uid_t) pwd->pw_uid);
+ if (setuid ((uid_t) pwd->pw_uid) < 0)
+ error (EXIT_FAILURE, errno, "failed to set user-ID");
+
if (chdir (pwd->pw_dir) < 0)
- {
- error ("No remote directory.\n");
- exit (1);
- }
+ error (EXIT_FAILURE, errno, "failed to change home directory");
+
strcat (path, PATH_DEFPATH);
environ = envinit;
strncat (homedir, pwd->pw_dir, sizeof (homedir) - 6);
@@ -342,23 +340,7 @@ doit (int f, struct sockaddr_in *fromp)
else
cp = pwd->pw_shell;
execl (pwd->pw_shell, cp, "-c", cmdbuf, 0);
- perror (pwd->pw_shell);
- exit (1);
-}
-
-void
-error (const char *fmt, ...)
-{
- va_list ap;
- char buf[BUFSIZ];
-#if defined(HAVE_STDARG_H) && defined(__STDC__) && __STDC__
- va_start (ap, fmt);
-#else
- va_start (ap);
-#endif
- buf[0] = 1;
- snprintf (buf + 1, sizeof buf - 1, fmt, ap);
- write (STDERR_FILENO, buf, strlen (buf));
+ error (EXIT_FAILURE, errno, "cannot execl %s", pwd->pw_shell);
}
char *
@@ -368,10 +350,7 @@ getstr (const char *err)
char *buf = malloc (buf_len), *end = buf;
if (!buf)
- {
- error ("Out of space reading %s\n", err);
- exit (1);
- }
+ error (EXIT_FAILURE, errno, "out of space reading %s", err);
do
{
@@ -380,10 +359,9 @@ getstr (const char *err)
if (rd <= 0)
{
if (rd == 0)
- error ("EOF reading %s\n", err);
+ error (EXIT_FAILURE, errno, "EOF reading");
else
- perror (err);
- exit (1);
+ error (EXIT_FAILURE, errno, "error reading");
}
end += rd;
@@ -394,10 +372,8 @@ getstr (const char *err)
buf_len += buf_len;
buf = realloc (buf, buf_len);
if (!buf)
- {
- error ("Out of space reading %s\n", err);
- exit (1);
- }
+ error (EXIT_FAILURE, errno, "out of space reading %", err);
+
end = buf + end_offs;
}
}
--
1.6.3.1
- [bug-inetutils] [PATCH] rexecd possible security problem, Giuseppe Scrivano, 2009/07/09
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Alfred M. Szmidt, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem,
Giuseppe Scrivano <=
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Giuseppe Scrivano, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Giuseppe Scrivano, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Alfred M. Szmidt, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Alfred M. Szmidt, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Sergey Poznyakoff, 2009/07/10
- Re: [bug-inetutils] [PATCH] rexecd possible security problem, Alfred M. Szmidt, 2009/07/10