Package: tinyproxy
Version: 1.8.3-3
Severity: important
Tags: upstream patch
Control: forwarded -1 https://bugzilla.banu.com/show_bug.cgi?id=115

When configured to change to a different user and group after startup
(User and Group configuration directives), tinyproxy does not drop
supplementary groups. These are still inherited from the calling
process. This may lead to a situation where tinyproxy has more access
than expected. At least on Debian systems root shells have "root" as a
supplementary group. This currently leads to the situation where
tinyproxy can read files readable only by the group "root".

The attached patch fixes this by dropping all supplementary groups if
the Group directive is set.

If only the User directive is set, groups are not dropped. This is
inline with the current code that also does not change the primary group
of the process in this case. I'm not sure if that's the behavior an
average user would expect. It might be safer to change to the primary
group of that user in this case and to either also drop supplementary
groups or change to the supplementary groups of this user with the
initgroups function.

The patch adds a new check for the setgroups function as this function
is not part of POSIX.

I also submitted this bug upstream as
https://bugzilla.banu.com/show_bug.cgi?id=115. But it would still be
useful to fix it with a patch in the Debian package until it's fixed
upstream.

Gaudenz

-- System Information:
Debian Release: jessie/sid
  APT prefers testing
  APT policy: (800, 'testing'), (700, 'unstable'), (50, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.10-2-amd64 (SMP w/2 CPU cores)
Locale: LANG=de_CH.UTF-8, LC_CTYPE=de_CH.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
>From cb0b7454c17dceb9be7ebb6b5f9e86c2a3389f36 Mon Sep 17 00:00:00 2001
From: Gaudenz Steinlin <gaud...@debian.org>
Date: Mon, 9 Sep 2013 08:33:48 +0200
Subject: [PATCH] Drop supplementary groups

Supplementary groups are inherited from the calling process. Drop all
supplementary groups if the "Group" configuration directive is set to
change to a different user. Otherwise the process may have more rights
than expected.
---
 configure.ac |  2 +-
 src/main.c   | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 303fc7b..377e204 100644
--- a/configure.ac
+++ b/configure.ac
@@ -203,7 +203,7 @@ AC_FUNC_REALLOC
 AC_CHECK_FUNCS([gethostname inet_ntoa memchr memset select socket strcasecmp \
                 strchr strdup strerror strncasecmp strpbrk strstr strtol])
 AC_CHECK_FUNCS([isascii memcpy setrlimit ftruncate regcomp regexec])
-AC_CHECK_FUNCS([strlcpy strlcat])
+AC_CHECK_FUNCS([strlcpy strlcat setgroups])
 
 
 dnl Enable extra warnings
diff --git a/src/main.c b/src/main.c
index a7ae9c7..abb047d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -296,6 +296,16 @@ change_user (const char *program)
                         exit (EX_NOPERM);
                 }
 
+#ifdef HAVE_SETGROUPS
+                /* Drop all supplementary groups, otherwise these are inherited from the calling process */
+                if (setgroups (0, NULL) < 0) {
+                        fprintf (stderr,
+                                 "%s: Unable to drop supplementary groups.\n",
+                                 program);
+                        exit (EX_NOPERM);
+                }
+#endif
+
                 log_message (LOG_INFO, "Now running as group \"%s\".",
                              config.group);
         }
-- 
1.8.4.rc3

Reply via email to