[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#684554: marked as done (unblock: rssh/2.3.3-5)



Your message dated Sat, 11 Aug 2012 10:23:44 +0100
with message-id <1344677024.2978.31.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#684554: unblock: rssh/2.3.3-5
has caused the Debian Bug report #684554,
regarding unblock: rssh/2.3.3-5
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
684554: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=684554
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package rssh.  This fixes a security vulnerability in
the command-line parsing by applying upstream's patch and then
adjusting the Debian patches accordingly.

unblock rssh/2.3.3-5

Because the package uses TopGit (I intend to switch to git-dpm) and
generates a quilt series, the straight debdiff is not particularly
useful since it's a diff between various patches.  Instead, attached
is the diff between the unpacked source for 2.3.3-4 and 2.3.3-5 with
all patches already applied, excluding the debian/patches directory
from the diff.  This seems like a better diff to review.  Let me know
if you want a diff in another format, however.

There are a few unfortunate whitespace-only hunks here that are due to
applying the upstream patch verbatim.  I considered removing them, but
decided that keeping the official upstream patch applied verbatim was
a better idea since it made for easier comparisons.

There is a stable security upload pending.  I've already contacted the
security team about that.

-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (990, 'testing'), (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 3.2.0-2-686-pae (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diff -ru --exclude .pc --exclude patches rssh-2.3.3-4/debian/changelog rssh-2.3.3-5/debian/changelog
--- rssh-2.3.3-4/debian/changelog	2012-03-07 16:08:52.000000000 -0800
+++ rssh-2.3.3-5/debian/changelog	2012-08-10 22:14:54.000000000 -0700
@@ -1,3 +1,11 @@
+rssh (2.3.3-5) unstable; urgency=medium
+
+  * Apply upstream patch to close security vulnerability that permitted
+    clever manipulation of environment variables on the ssh command line
+    to bypass rssh checking.  (CVE-2012-3478)
+
+ -- Russ Allbery <rra@debian.org>  Fri, 10 Aug 2012 22:14:34 -0700
+
 rssh (2.3.3-4) unstable; urgency=low
 
   * Force libexecdir to /usr/lib/rssh.  This is not a library package and
diff -ru --exclude .pc --exclude patches rssh-2.3.3-4/main.c.in rssh-2.3.3-5/main.c.in
--- rssh-2.3.3-4/main.c.in	2012-08-09 18:21:02.000000000 -0700
+++ rssh-2.3.3-5/main.c.in	2012-08-10 22:46:01.000000000 -0700
@@ -184,7 +184,7 @@
 	 * determine if the command in cmdline is acceptable to run, and store
 	 * name of program to exec in cmd
 	 */
-	if ( !(*cmd = check_command_line(cmdline, opts)) ) return NULL;
+	if ( !(*cmd = get_command(cmdline, opts)) ) return NULL;
 
 	/* if we need to do chroot processing, do it */
 	if ( opts->shell_flags & RSSH_USE_CHROOT ){
@@ -254,7 +254,9 @@
 	}
 
 	/* return vector of pointers to command line arguments */
-	return build_arg_vector(cmdline, 0);
+	argvec = build_arg_vector(cmdline, 0);
+	if (check_command_line(argvec, opts)) return argvec;
+	else return NULL;
 }
 
 void vers_info( void )
diff -ru --exclude .pc --exclude patches rssh-2.3.3-4/util.c rssh-2.3.3-5/util.c
--- rssh-2.3.3-4/util.c	2012-08-09 18:21:02.000000000 -0700
+++ rssh-2.3.3-5/util.c	2012-08-10 22:46:01.000000000 -0700
@@ -110,7 +110,7 @@
 	/* print error message to user and log attempt */
 	fprintf(stderr, "\nThis account is restricted by rssh.\n"
 		"%s\n\nIf you believe this is in error, please contact "
-	        "your system administrator.\n\n", cmd);
+		"your system administrator.\n\n", cmd);
 	if ( argc < 3 )
 		log_msg("user %s attempted to log in with a shell",
 			username);
@@ -136,31 +136,35 @@
  */
 bool opt_exist(char *cl, char opt)
 {
-	int	i = 0;
+	int	i = 1;
 	int	len;
-	char	*token;
-	bool	optstring = FALSE;
-
 
 	len = strlen(cl);
 
 	/* process command line character by character */
-	while ( i < (len - 2) ){
-		if ( cl[i] == ' ' || cl[i] == '\t' ){
-			if ( cl[i+1] == '-' ){ 
-				optstring = TRUE;
-				i+=2;
-			}
-		}
-		if ( cl[i] == opt && optstring ) return TRUE;
-		if ( cl[i] == ' ' || cl[i] == '\t' || cl[i] == '-' ) 
-			optstring = FALSE;
+	if (!(cl[0] == '-')) return FALSE;
+	while ( i < (len) ){
+		if ( cl[i] == opt ) return TRUE;
 		i++;
 	}
 	return FALSE;
 }
 
 
+bool opt_filter(char **vec, const char opt)
+{
+	while (vec && *vec){
+		if (opt_exist(*vec, opt)){
+			fprintf(stderr, "\nillegal insecure %c option", opt);
+			log_msg("insecure %c option in command line!", opt);
+			return TRUE;
+		}
+		vec++;
+	}
+	return FALSE;
+}
+
+
 bool check_command( char *cl, ShellOptions_t *opts, char *cmd, int cmdflag )
 {
 	int	cl_len;		/* length of command line */
@@ -190,95 +194,98 @@
 	return FALSE;
 }
 
+
 /*
  * check_rsync_e() - take the command line passed to rssh and look for a -e
- *                   option.  If one is found, make sure --server is provided
- *                   and the option contains only the protocol information.
- *                   Returns 1 if the command line is safe; 0 otherwise.
+ *		     option.  If one is found, make sure --server is provided
+ *		     and the option contains only the protocol information.
+ *		     Returns FALSE if the command line should not be allowed,
+ *		     TRUE if it is okay.
  */
-static int check_rsync_e( char *cl )
+static int rsync_e_okay( char **vec )
 {
 	int	status;
 	regex_t	re;
+	int	server = FALSE;
+	int	e_found = FALSE;
+
+	static const char pattern[] = "^-([^-][^ ]*)?e[^.0-9]";
 
 	/*
 	 * This is more complicated than it looks because we don't want to
-	 * trigger on the e in --server, but we do want to catch the common
+	 * trigger on the e in --server, but we do want to allow the common
 	 * case of -ltpre.iL (which contains -e.).
 	 */
-	static const char pattern[] = "[ \t\v\f]-([^-][^ ]*)?e[^.0-9]";
-
-	if ( strstr(cl, "--server") == NULL ) return 0;
 	if ( regcomp(&re, pattern, REG_EXTENDED | REG_NOSUB) != 0 ){
-		return 0;
+		return FALSE;
+	}
+	while (vec && *vec){
+		if ( strcmp(*vec, "--server") == 0 ) server = TRUE;
+		if ( strncmp(*vec, "--", 2) != 0 && opt_exist(*vec, 'e') ){
+			e_found = TRUE;
+			if ( regexec(&re, *vec, 0, NULL, 0) == 0 ){
+				regfree(&re);
+				return FALSE;
+			}
+		}
+		vec++;
 	}
-	status = regexec(&re, cl, 0, NULL, 0);
 	regfree(&re);
-	return (status == 0) ? 0 : 1;
+	if ( e_found && !server ) return FALSE;
+	return TRUE;
 }
 
+
 /*
  * check_command_line() - take the command line passed to rssh, and verify
- * 			  that the specified command is one the user is
- * 			  allowed to run.  Return the path of the command
- * 			  which will be run if it is ok, or return NULL if it
- * 			  is not.
+ *			  that the specified command is one the user is
+ *			  allowed to run and validate the arguments.  Return the
+ *			  path of the command which will be run if it is ok, or
+ *			  return NULL if it is not.
  */
-char *check_command_line( char *cl, ShellOptions_t *opts )
+char *check_command_line( char **cl, ShellOptions_t *opts )
 {
 
-	if ( check_command(cl, opts, PATH_SFTP_SERVER, RSSH_ALLOW_SFTP) )
+	if ( check_command(*cl, opts, PATH_SFTP_SERVER, RSSH_ALLOW_SFTP) )
 		return PATH_SFTP_SERVER;
 
-	if ( check_command(cl, opts, PATH_SCP, RSSH_ALLOW_SCP) ){
+	if ( check_command(*cl, opts, PATH_SCP, RSSH_ALLOW_SCP) ){
 		/* filter -S option */
-		if ( opt_exist(cl, 'S') ){
-			fprintf(stderr, "\ninsecure -S option not allowed.");
-			log_msg("insecure -S option in scp command line!");
-			return NULL;
-		}
+		if ( opt_filter(cl, 'S') ) return NULL;
 		return PATH_SCP;
 	}
 
-	if ( check_command(cl, opts, PATH_CVS, RSSH_ALLOW_CVS) ){
-		if ( opt_exist(cl, 'e') ){
-			fprintf(stderr, "\ninsecure -e option not allowed.");
-			log_msg("insecure -e option in cvs command line!");
-			return NULL;
-		}
+	if ( check_command(*cl, opts, PATH_CVS, RSSH_ALLOW_CVS) ){
+		if ( opt_filter(cl, 'e') ) return NULL;
 		return PATH_CVS;
 	}
 
-	if ( check_command(cl, opts, PATH_RDIST, RSSH_ALLOW_RDIST) ){
+	if ( check_command(*cl, opts, PATH_RDIST, RSSH_ALLOW_RDIST) ){
 		/* filter -P option */
-		if ( opt_exist(cl, 'P') ){
-			fprintf(stderr, "\ninsecure -P option not allowed.");
-			log_msg("insecure -P option in rdist command line!");
-			return NULL;
-		}
+		if ( opt_filter(cl, 'P') ) return NULL;
 		return PATH_RDIST;
 	}
 
-	if ( check_command(cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){
+	if ( check_command(*cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){
 		/* filter -e option */
-		if ( opt_exist(cl, 'e') && !check_rsync_e(cl) ){
-			fprintf(stderr, "\ninsecure -e option not allowed.");
-			log_msg("insecure -e option in rsync command line!");
+		if ( !rsync_e_okay(cl) ){
+			fprintf(stderr, "\nillegal insecure e option");
+			log_msg("insecure e option in rsync command line!");
 			return NULL;
 		}
-		
-		if ( strstr(cl, "--rsh=" ) ){
-			fprintf(stderr, "\ninsecure --rsh= not allowed.");
-			log_msg("insecure --rsh option in rsync command line!");
-			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++;
 		}
-
 		return PATH_RSYNC;
 	}
-
-	if ( check_command(cl, opts, PATH_SVNSERVE, RSSH_ALLOW_SVNSERVE) ){
+	if ( check_command(*cl, opts, PATH_SVNSERVE, RSSH_ALLOW_SVNSERVE) ){
 		/* check command line */
-		if( strlen(cl) != 11 || strcmp(cl, "svnserve -t") ) {
+		if( cl[1] == NULL || strcmp(cl[1], "-t") != 0 || cl[2] != NULL){
 			fprintf(stderr, "\nextra svnserver parameter(s) not allowed.");
 			log_msg("unallowed option(s) in svnserver command line!");
 			return NULL;
@@ -286,11 +293,37 @@
 
 		return PATH_SVNSERVE;
 	}
+	/* No match, return NULL */
+	return NULL;
+}
+
+
+/*
+ * get_command() - take the command line passed to rssh, and verify
+ *		   that the specified command is one the user is allowed to run.
+ *		   Return the path of the command which will be run if it is ok,
+ *		   or return NULL if it is not.
+ */
+char *get_command( char *cl, ShellOptions_t *opts )
+{
 
+	if ( check_command(cl, opts, PATH_SFTP_SERVER, RSSH_ALLOW_SFTP) )
+		return PATH_SFTP_SERVER;
+	if ( check_command(cl, opts, PATH_SCP, RSSH_ALLOW_SCP) )
+		return PATH_SCP;
+	if ( check_command(cl, opts, PATH_CVS, RSSH_ALLOW_CVS) )
+		return PATH_CVS;
+	if ( check_command(cl, opts, PATH_RDIST, RSSH_ALLOW_RDIST) )
+		return PATH_RDIST;
+	if ( check_command(cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) )
+		return PATH_RSYNC;
+	if ( check_command(cl, opts, PATH_SVNSERVE, RSSH_ALLOW_SVNSERVE) )
+		return PATH_SVNSERVE;
 	return NULL;
 }
 
 
+
 /*
  * extract_root() - takes a root directory and the full path to some other
  *                  directory, and returns a pointer to a string which
@@ -306,7 +339,7 @@
 	len = strlen(root);
 	/* get rid of a trailing / from the root path */
 	if ( root[len - 1] == '/' ){
-	       	root[len - 1] = '\0';
+		root[len - 1] = '\0';
 		len--;
 	}
 	if ( (strncmp(root, path, len)) ) return NULL;
diff -ru --exclude .pc --exclude patches rssh-2.3.3-4/util.h rssh-2.3.3-5/util.h
--- rssh-2.3.3-4/util.h	2012-08-09 18:21:02.000000000 -0700
+++ rssh-2.3.3-5/util.h	2012-08-10 22:46:01.000000000 -0700
@@ -33,7 +33,8 @@
 #include "rsshconf.h"
 
 void fail( int flags, int argc, char **argv );
-char *check_command_line( char *cl, ShellOptions_t *opts );
+char *check_command_line( char **cl, ShellOptions_t *opts );
+char *get_command( char *cl, ShellOptions_t *opts);
 char *extract_root( char *root, char *path );
 int  validate_umask( const char *temp, int *mask );
 int validate_access( const char *temp, bool *allow_sftp, bool *allow_scp,

--- End Message ---
--- Begin Message ---
On Fri, 2012-08-10 at 22:47 -0700, Russ Allbery wrote:
> Please unblock package rssh.  This fixes a security vulnerability in
> the command-line parsing by applying upstream's patch and then
> adjusting the Debian patches accordingly.

Unblocked; thanks.

Regards,

Adam

--- End Message ---

Reply to: