Bug#694577: unblock: rssh/2.3.3-6
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
Please unblock package rssh for a security update. The changelog:
* Fix several flaws in validation of rsync options. Ensure --server
cannot be hidden from the server by putting it after -- or as the
argument to another option. Verify that the -e option's value matches
expectations rather than trying to look for invalid -e option values.
(CVE-2012-2251)
* Reject the rsync --rsh option even if it does not contain a trailing
equal sign. (CVE-2012-2252)
This corresponds to the just-issued DSA for stable. The debdiff is
attached (which is unfortunately a diff of patches; sorry about that).
unblock rssh/2.3.3-6
-- 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-3-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 -Nru rssh-2.3.3/debian/changelog rssh-2.3.3/debian/changelog
--- rssh-2.3.3/debian/changelog 2012-08-10 22:14:54.000000000 -0700
+++ rssh-2.3.3/debian/changelog 2012-11-22 12:15:02.000000000 -0800
@@ -1,3 +1,15 @@
+rssh (2.3.3-6) unstable; urgency=high
+
+ * Fix several flaws in validation of rsync options. Ensure --server
+ cannot be hidden from the server by putting it after -- or as the
+ argument to another option. Verify that the -e option's value matches
+ expectations rather than trying to look for invalid -e option values.
+ (CVE-2012-2251)
+ * Reject the rsync --rsh option even if it does not contain a trailing
+ equal sign. (CVE-2012-2252)
+
+ -- Russ Allbery <rra@debian.org> Thu, 22 Nov 2012 12:01:41 -0800
+
rssh (2.3.3-5) unstable; urgency=medium
* Apply upstream patch to close security vulnerability that permitted
diff -Nru rssh-2.3.3/debian/patches/features/subversion.diff rssh-2.3.3/debian/patches/features/subversion.diff
--- rssh-2.3.3/debian/patches/features/subversion.diff 2012-08-10 22:14:54.000000000 -0700
+++ rssh-2.3.3/debian/patches/features/subversion.diff 2012-11-22 12:15:02.000000000 -0800
@@ -517,7 +517,7 @@
if (log) log_msg("chrooting %s to %s", user, path);
opts->shell_flags |= RSSH_USE_CHROOT;
diff --git a/util.c b/util.c
-index a3e9829..74b22f0 100644
+index 24a8ab3..438c3c9 100644
--- a/util.c
+++ b/util.c
@@ -80,7 +80,8 @@ void fail( int flags, int argc, char **argv )
@@ -541,7 +541,7 @@
}
/* print error message to user and log attempt */
-@@ -280,6 +283,16 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
+@@ -299,6 +302,16 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
}
return PATH_RSYNC;
}
@@ -558,7 +558,7 @@
/* No match, return NULL */
return NULL;
}
-@@ -304,6 +317,8 @@ char *get_command( char *cl, ShellOptions_t *opts )
+@@ -323,6 +336,8 @@ char *get_command( char *cl, ShellOptions_t *opts )
return PATH_RDIST;
if ( check_command(cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) )
return PATH_RSYNC;
@@ -567,7 +567,7 @@
return NULL;
}
-@@ -369,22 +384,24 @@ int validate_umask( const char *temp, int *mask )
+@@ -388,22 +403,24 @@ int validate_umask( const char *temp, int *mask )
* same name, and returns FALSE if the bits are not valid
*/
int validate_access( const char *temp, bool *allow_sftp, bool *allow_scp,
@@ -614,4 +614,4 @@
char *get_username( void );
--
-tg: (7579edb..) features/subversion (depends on: fixes/info-to-debug fixes/rsync-protocol fixes/command-line-checking)
+tg: (b4a9b30..) features/subversion (depends on: fixes/info-to-debug fixes/rsync-protocol fixes/command-line-checking)
diff -Nru rssh-2.3.3/debian/patches/fixes/rsync-protocol.diff rssh-2.3.3/debian/patches/fixes/rsync-protocol.diff
--- rssh-2.3.3/debian/patches/fixes/rsync-protocol.diff 2012-08-10 22:14:54.000000000 -0700
+++ rssh-2.3.3/debian/patches/fixes/rsync-protocol.diff 2012-11-22 12:15:02.000000000 -0800
@@ -6,6 +6,11 @@
options to rsync, only ones not sent with --server or containing
something other than protocol information as an argument.
+Also scan the rsync command line for any --rsh 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.
+
Based on work by Robert Hardy.
Debian Bug#471803
@@ -13,11 +18,11 @@
Signed-off-by: Russ Allbery <rra@stanford.edu>
---
- util.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
- 1 file changed, 48 insertions(+), 1 deletion(-)
+ util.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
+ 1 file changed, 73 insertions(+), 7 deletions(-)
diff --git a/util.c b/util.c
-index 443dcba..a3e9829 100644
+index 443dcba..24a8ab3 100644
--- a/util.c
+++ b/util.c
@@ -56,6 +56,7 @@
@@ -28,15 +33,16 @@
/* LOCAL INCLUDES */
#include "pathnames.h"
-@@ -192,6 +193,47 @@ bool check_command( char *cl, ShellOptions_t *opts, char *cmd, int cmdflag )
+@@ -192,6 +193,74 @@ bool check_command( char *cl, ShellOptions_t *opts, char *cmd, int cmdflag )
/*
-+ * 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 FALSE if the command line should not be allowed,
-+ * TRUE if it is okay.
++ * rsync_e_okay() - take the command line passed to rssh and look for an -e
++ * option. If one is found, make sure --server is provided
++ * and the option contains only the protocol information.
++ * Also check for and reject any --rsh option. Returns FALSE
++ * if the command line should not be allowed, TRUE if it is
++ * okay.
+ */
+static int rsync_e_okay( char **vec )
+{
@@ -45,21 +51,47 @@
+ int server = FALSE;
+ int e_found = FALSE;
+
-+ static const char pattern[] = "^-([^-][^ ]*)?e[^.0-9]";
++ /*
++ * 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]*$";
+
+ /*
-+ * This is more complicated than it looks because we don't want to
-+ * trigger on the e in --server, but we do want to allow the common
-+ * case of -ltpre.iL (which contains -e.).
++ * 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 ){
++ server = TRUE;
++ }
++
++ /* 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, "--server") == 0 ) server = TRUE;
++ if ( strcmp(*vec, "--") == 0 ) break;
++ if ( strcmp(*vec, "--rsh") == 0
++ || strncmp(*vec, "--rsh=", strlen("--rsh=")) == 0 ){
++ regfree(&re);
++ return FALSE;
++ }
+ if ( strncmp(*vec, "--", 2) != 0 && opt_exist(*vec, 'e') ){
+ e_found = TRUE;
-+ if ( regexec(&re, *vec, 0, NULL, 0) == 0 ){
++ if ( regexec(&re, *vec, 0, NULL, 0) != 0 ){
+ regfree(&re);
+ return FALSE;
+ }
@@ -76,23 +108,21 @@
* check_command_line() - take the command line passed to rssh, and verify
* that the specified command is one the user is
* allowed to run and validate the arguments. Return the
-@@ -223,13 +265,18 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
+@@ -223,13 +292,10 @@ 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;
+- }
+ if ( !rsync_e_okay(cl) ){
-+ fprintf(stderr, "\nillegal insecure e option");
-+ log_msg("insecure e option in rsync command line!");
++ fprintf(stderr, "\ninsecure -e or --rsh option not allowed.");
++ log_msg("insecure -e or --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;
}
Reply to: