Hi all,

We've just released another security update for the rssh package in
Debian, this time for vulnerabilities in the rsync support.  These bugs
will also affect most other installations of rssh if rsync is enabled.
Both problems were discovered by Nick Cleaton.

CVE-2019-3463

    The client could send the --daemon and --config options to the server
    and they would be passed through by rssh.  Not only does this allow
    the client to start a daemon listening on the normal rsync port, which
    is probably not desirable, but various options set in the daemon
    configuration file specified with --config allow arbitrary code
    execution.  (The most obvious is pre-xfer exec.)

CVE-2019-3463

    If rsync is built with popt (as it is in Debian), the popt library
    will attempt to open and parse ~/.popt on the server and interpret it
    as configuration for command line option parsing.  If the client can
    rsync a .popt file to the home directory of the user protected with
    rssh, this allows arbitrary code execution on the server via several
    paths.  The most obvious is via the popt exec feature which execs an
    arbitrary program as an external option filter, but a more sneaky
    approach is through the popt alias feature which allows one to alias a
    benign option (such as --server) to one that will run arbitrary code
    (such as --rsh).

The second vulnerability is an excellent example of how hard this problem
is and how the tools that rssh is attempting to provide are working
directly against its security model.  I personally had no idea that popt
even existed, having missed it in the rsync man page (which is rather
long) and not deeply investigated the shared library dependencies of
rsync.  But even if I'd known it existed, the exec feature is rather
inobvious and unexpected, as is the interaction with aliases.

The workaround for this popt issue is to unset the HOME environment
variable, which disables popt's attempt to load ~/.popt in at least the
version currently in Debian.  Note that this is fragile: if the popt
developers ever decide to fall back on getpwnam to find the home
directory, this will stop working.  A safer approach (which can't easily
be done in rssh itself) is to ensure that the home directory of an rssh
user is not writable by that user.

Attached is an updated patch for the rsync support that incorporates fixes
for these two problems, as well as a separate problem where one could hide
insecure options from rssh by passing "--" as the *argument* to some other
option that takes an argument, causing rssh to stop parsing for options
but rsync to continue parsing options.  This means that -- can no longer
be used to stop rssh's option analysis, but other normal workarounds can
be used, such as prepending ./ to paths that start with -.

As mentioned previously, I think the takeaway is that we all need to stop
using rssh and find a more robust solution.  But I'll continue to provide
security support for the Debian stable package over the lifetime of Debian
stretch if possible, since that was the commitment I made when I uploaded
the package to Debian.

-- 
Russ Allbery (ea...@eyrie.org)              <http://www.eyrie.org/~eagle/>

From: Russ Allbery <r...@debian.org>
Date: Sat, 7 Dec 2013 18:32:56 -0800
Subject: Verify rsync command options

As of rsync 3, rsync reused the -e option to pass protocol information
from the client to the server.  We therefore cannot reject all -e
options to rsync, only ones not sent with --server or containing
something other than protocol information as an argument.

Be stricter about the rsync command line and require --server as the
first argument, which disables attempts to initiate rsync outbound from
the server and in turn could trigger running code specified in ssh
client configuration options.

Also scan the rsync command line for any --rsh, --config, or --daemon
option and reject it as well.  This replaces and improves the upstream
strategy for rejecting that command-line option, taking advantage of
the parsing added to check the -e option.  --config can be used to run
commands via "pre-xfer exec" when running as a daemon, plus the client
should not be able to spawn daemons.

Unset the HOME environment variable to prevent popt from loading a
~/.popt configuration file, which could redefine rsync command-line
options like --server to instead mean some unsafe option, or even run
commands directly.

Based on work by Robert Hardy and a report by Nick Cleaton.

Debian Bug#471803
---
 util.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 9 deletions(-)

diff --git a/util.c b/util.c
index ef1a5d8..760e4c7 100644
--- a/util.c
+++ b/util.c
@@ -56,6 +56,7 @@
 #ifdef HAVE_LIBGEN_H
 #include <libgen.h>
 #endif /* HAVE_LIBGEN_H */
+#include <regex.h>
 
 /* LOCAL INCLUDES */
 #include "pathnames.h"
@@ -197,6 +198,68 @@ bool check_command( char *cl, ShellOptions_t *opts, char *cmd, int cmdflag )
 }
 
 
+/*
+ * rsync_okay() - require --server on all rsh command lines, check that -e
+ *		  contains only protocol information, and reject any --rsh,
+ *		  --config, or --daemon option. Returns FALSE if the command
+ *		  line should not be allowed, TRUE if it is okay.
+ */
+static int rsync_okay( char **vec )
+{
+	regex_t	re;
+
+	/*
+	 * rsync will send -e, followed by either just "." (meaning no special
+	 * protocol) or "N.N" (meaning a pre-release protocol version),
+	 * followed by some number of alphabetic flags indicating various
+	 * supported options.  There may be other options between - and the e,
+	 * but -e will always be the last option in the string.	 A typical
+	 * option passed by the client is "-ltpre.iL".
+	 *
+	 * Note that if --server is given, this should never be parsed as a
+	 * shell, but we'll tightly verify it anyway, just in case.
+	 *
+	 * This regex matches the acceptable flags containing -e, so if it
+	 * does not match, the command line should be rejected.
+	 */
+	static const char pattern[]
+	    = "^-[a-df-zA-Z]*e[0-9]*\\.[0-9]*[a-zA-Z]*$";
+
+	/*
+	 * Only recognize --server if it's the first option.  rsync itself
+	 * always passes it that way, and if it's not the first argument, it
+	 * could be hidden from the server as an argument to some other
+	 * option.
+	 */
+	if ( !(vec && vec[0] && vec[1] && strcmp(vec[1], "--server") == 0) )
+		return FALSE;
+
+	/* Check the remaining options for -e or --rsh. */
+	if ( regcomp(&re, pattern, REG_EXTENDED | REG_NOSUB) != 0 ){
+		return FALSE;
+	}
+	while (vec && *vec){
+		if ( strcmp(*vec, "--rsh") == 0
+		     || strcmp(*vec, "--daemon") == 0
+		     || strcmp(*vec, "--config") == 0
+		     || strncmp(*vec, "--rsh=", strlen("--rsh=")) == 0
+		     || strncmp(*vec, "--config=", strlen("--config=")) == 0 ){
+			regfree(&re);
+			return FALSE;
+		}
+		if ( strncmp(*vec, "--", 2) != 0 && opt_exist(*vec, 'e') ){
+			if ( regexec(&re, *vec, 0, NULL, 0) != 0 ){
+				regfree(&re);
+				return FALSE;
+			}
+		}
+		vec++;
+	}
+	regfree(&re);
+	return TRUE;
+}
+
+
 /*
  * check_command_line() - take the command line passed to rssh, and verify
  *			  that the specified command is one the user is
@@ -229,16 +292,27 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
 	}
 
 	if ( check_command(*cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){
-		/* filter -e option */
-		if ( opt_filter(cl, 'e') ) return NULL;
-		while (cl && *cl){
-			if ( strstr(*cl, "--rsh" ) ){
-				fprintf(stderr, "\ninsecure --rsh= not allowed.");
-				log_msg("insecure --rsh option in rsync command line!");
-				return NULL;
-			}
-			cl++;
+		if ( !rsync_okay(cl) ){
+			fprintf(stderr, "\ninsecure rsync options not allowed.");
+			log_msg("insecure rsync options in rsync command line!");
+			return NULL;
 		}
+
+		/*
+		 * rsync is linked with popt, which recognizes a configuration
+		 * file ~/.popt that can, among other things, define aliases.
+		 * If someone can write to the home directory of the rssh
+		 * user, they can upload a ~/.popt file that contains
+		 * something like "rsync alias --server --rsh" and then
+		 * execute commands they upload.  popt does not try to read
+		 * its configuration file if HOME is not set, so unset HOME to
+		 * disable this behavior.
+		 */
+		if ( unsetenv("HOME") < 0 ){
+			log_msg("cannot unsetenv() HOME");
+			return NULL;
+		}
+
 		return PATH_RSYNC;
 	}
 	/* No match, return NULL */
_______________________________________________
rssh-discuss mailing list
rssh-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rssh-discuss

Reply via email to