Blob Blame History Raw
From: Russ Allbery <rra@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 */