Attachment #202528 for bug #235121

Lines 3-9 Link Here

(-)shells/rssh/Makefile (-1 / +1 lines)
3		3
4	PORTNAME= rssh	4	PORTNAME= rssh
5	PORTVERSION= 2.3.4	5	PORTVERSION= 2.3.4
6	PORTREVISION= 1	6	PORTREVISION= 2
7	CATEGORIES= shells security	7	CATEGORIES= shells security
8	MASTER_SITES= SF	8	MASTER_SITES= SF
9		9




Verifies the command line options for rysnc. This is an updated version that
tightens the argument checking and requires to run rsync in server mode.
Taken from Debian ("0007-Verify-rsync-command-options").
--- util.c.orig	2012-11-27 01:14:49 UTC
+++ util.c
@@ -56,6 +56,7 @@
 #ifdef HAVE_LIBGEN_H
 #include <libgen.h>

 
 /* LOCAL INCLUDES */
 #include "pathnames.h"
@@ -198,6 +199,71 @@ bool check_command( char *cl, ShellOptions_t *opts, ch
 
 
 /*
+ * 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.
+ *		    if the command line should not be allowed, TRUE if it is
+ *		    okay.
+ */
+static int rsync_okay( char **vec )
+{
+	regex_t	re;
+	int	server = FALSE;

+	 * 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, "--") == 0 ) break;
+		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;
+		}

+		vec++;
+	}
+	regfree(&re);
+	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 and validate the arguments.  Return the
@@ -229,16 +295,27 @@ char *check_command_line( char **cl, ShellOptions_t *o
 	}
 
 	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" ) ){

-				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 */




Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12
--- log.c.orig	2012-11-27 00:25:13 UTC
+++ log.c
@@ -93,10 +93,14 @@ char *log_make_ident( const char *name )
 	}
 	/* assign new value to ident from name */
 	if ( !name ) return (ident = NULL);
-	ident = strdup(basename((char*)name));
-	/* remove leading '-' from ident, if there is one */
-	if ( ident[0] == '-' ){
-		temp = strdup(ident + 1);
+	/* clone name in case basename() is POSIX-compliant */
+	temp = strdup ((char *) name);
+	/* always pass writeable string to basename() */
+	ident = strdup (basename (temp));
+	free (temp);
+	/* safely remove leading '-' from ident, if there is one */
+	if ((ident != NULL) && (ident[0] == '-')){
+		temp = strdup(&ident[1]);
 		free(ident);
 		ident = temp;
 	}




Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12

Incorporates also a patch to check the command line after chroot. Taken from
Debian ("0010-Check-command-line-after-chroot.patch")

--- rssh_chroot_helper.c.orig	2006-12-21 22:22:35 UTC
+++ rssh_chroot_helper.c
@@ -159,7 +159,7 @@ int main( int argc, char **argv )
 	opts.chroot_path = NULL;
 
 	/* figure out our name, and give it to the log module */
-	progname = strdup(log_make_ident(basename(argv[0])));
+	progname = strdup(log_make_ident(basename(strdup (argv[0]))));
 
 	/* get user's passwd info */
 	if ( (temp = getpwuid(getuid())) ){
@@ -217,6 +217,12 @@ int main( int argc, char **argv )
 	if ( !(argvec = build_arg_vector(argv[2], 0)) )
 		ch_fatal_error("build_arg_vector()", argv[2],
 				"bad expansion");
+
+	/* check the command for safety */
+	if ( !check_command_line(argvec, &opts) ){
+		fprintf(stderr, "\n");
+		exit(1);
+	}
 
 	/* 
 	 * This is the old way to figure out what program to run.  Since we're




Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12

Fixes buffer allocation for the fail message. Taken from Debian
("0003-Fix-buffer-allocation-buffer-for-fail-message").

Tightens the check for scp command line arguments that fixes also
"CVE-2019-1000018". Taken from Debian ("0009-Verify-scp-command-options").
Please note that with this patch the scp option "-3" can no longer be used.

--- util.c.orig	2012-11-27 01:14:49 UTC
+++ util.c
@@ -84,7 +84,7 @@ void fail( int flags, int argc, char **argv )
 	/* create msg indicating what is allowed */
 	if ( !size ) cmd = "This user is locked out.";
 	else {
-		size += 18;
+		size += 18 + 1;
 		if ( !(cmd = (char *)malloc(size)) ){
 			log_msg("fatal error: out of mem allocating log msg");
 			exit(1);
@@ -165,6 +165,7 @@ bool check_command( char *cl, ShellOptions_t *opts, ch
 {
 	char	*prog;		/* basename of cmd */
 	char 	*tmp = cl;
+	char    *tmp2 = NULL;
 	bool	need_free = FALSE;
 	bool	rc = FALSE;
 	int 	i;
@@ -186,11 +187,17 @@ bool check_command( char *cl, ShellOptions_t *opts, ch
 		}
 
 		/* compare tmp to cmd and prog for match */
-		prog = basename(cmd);
+		tmp2 = strdup (cmd);
+		if (tmp2 == NULL) {
+			log_msg ("strdup() failed in check_command()");
+			return FALSE;
+		}
+		prog = basename(tmp2);
 		if ( !(strcmp(tmp, cmd)) || !(strcmp(tmp, prog))){
 			log_msg("cmd '%s' approved", prog);
 			rc = TRUE;
 		}	
+		free (tmp2);
 	}
 	if (need_free) free(tmp);
 	return rc;
@@ -198,6 +205,43 @@ bool check_command( char *cl, ShellOptions_t *opts, ch
 
 
 /*
+ * scp_okay() - take the command line and check that it is a hopefully-safe scp
+ *		server command line, accepting only very specific options.
+ *		Returns FALSE if the command line should not be allowed, TRUE
+ *		if it is okay.
+ */
+static int scp_okay( char **vec )
+{
+	int saw_f_or_t = FALSE;
+
+	for ( vec++; vec && *vec; vec++ ){
+		/* Allowed options. */
+		if ( strcmp(*vec, "-v") == 0 ) continue;
+		if ( strcmp(*vec, "-r") == 0 ) continue;
+		if ( strcmp(*vec, "-p") == 0 ) continue;
+		if ( strcmp(*vec, "-d") == 0 ) continue;
+		if ( strcmp(*vec, "-f") == 0 || strcmp(*vec, "-pf") == 0 ){
+			saw_f_or_t = TRUE;
+			continue;
+		}
+		if ( strcmp(*vec, "-t") == 0 || strcmp(*vec, "-pt") == 0 ){
+			saw_f_or_t = TRUE;
+			continue;
+		}
+
+		/* End of arguments. */
+		if ( strcmp(*vec, "--") == 0 ) break;
+
+		/* Any other argument is not allowed. */
+		if ( *vec[0] == '-' ) return FALSE;
+	}
+
+	/* Either -f or -t must have been given. */
+	return saw_f_or_t;
+}
+
+
+/*
  * 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
@@ -212,8 +256,11 @@ char *check_command_line( char **cl, ShellOptions_t *o
 		return PATH_SFTP_SERVER;
 
 	if ( check_command(*cl, opts, PATH_SCP, RSSH_ALLOW_SCP) ){
-		/* filter -S option */
-		if ( opt_filter(cl, 'S') ) return NULL;
+		if ( !scp_okay(cl) ){
+			fprintf(stderr, "\ninsecure scp option not allowed.");
+			log_msg("insecure scp option in scp command line");
+			return NULL;
+		}
 		return PATH_SCP;
 	}
 

Return to bug 235121

Lines 1-5 Link Here

(-)shells/rssh/files/optional-patch-util.c (-21 / +40 lines)
1	--- util.c.orig 2012-11-27 12:14:49.000000000 +1100	1	Verifies the command line options for rysnc. This is an updated version that
2	+++ util.c 2013-01-09 17:52:54.000000000 +1100	2	tightens the argument checking and requires to run rsync in server mode.
		3	Taken from Debian ("0007-Verify-rsync-command-options").
		4	--- util.c.orig 2012-11-27 01:14:49 UTC
		5	+++ util.c
3	@@ -56,6 +56,7 @@	6	@@ -56,6 +56,7 @@
4	#ifdef HAVE_LIBGEN_H	7	#ifdef HAVE_LIBGEN_H
5	#include <libgen.h>	8	#include <libgen.h>
Lines 8-25 Link Here
8		11
9	/* LOCAL INCLUDES */	12	/* LOCAL INCLUDES */
10	#include "pathnames.h"	13	#include "pathnames.h"
11	@@ -198,6 +199,73 @@	14	@@ -198,6 +199,71 @@ bool check_command( char cl, ShellOptions_t opts, ch
12		15
13		16
14	/*	17	/*
15	+ * rsync_e_okay() - take the command line passed to rssh and look for an -e	18	+ * rsync_okay() - require --server on all rsh command lines, check that -e
16	+ * option. If one is found, make sure --server is provided	19	+ * contains only protocol information, and reject any --rsh,
17	+ * and the option contains only the protocol information.	20	+ * --config, or --daemon option. Returns FALSE if the command
18	+ * Also check for and reject any --rsh option. Returns FALSE	21	+ * line should not be allowed, TRUE if it is okay.
19	+ * if the command line should not be allowed, TRUE if it is
20	+ * okay.
21	+ */	22	+ */
22	+static int rsync_e_okay( char **vec )	23	+static int rsync_okay( char **vec )
23	+{	24	+{
24	+ regex_t re;	25	+ regex_t re;
25	+ int server = FALSE;	26	+ int server = FALSE;
Lines 48-56 Link Here
48	+ * could be hidden from the server as an argument to some other	49	+ * could be hidden from the server as an argument to some other
49	+ * option.	50	+ * option.
50	+ */	51	+ */
51	+ if ( vec && vec[0] && vec[1] && strcmp(vec[1], "--server") == 0 ){	52	+ if ( !(vec && vec[0] && vec[1] && strcmp(vec[1], "--server") == 0) )
52	+ server = TRUE;	53	+ return FALSE;
53	+ }
54	+	54	+
55	+ /* Check the remaining options for -e or --rsh. */	55	+ /* Check the remaining options for -e or --rsh. */
56	+ if ( regcomp(&re, pattern, REG_EXTENDED \| REG_NOSUB) != 0 ){	56	+ if ( regcomp(&re, pattern, REG_EXTENDED \| REG_NOSUB) != 0 ){
Lines 57-65 Link Here
57	+ return FALSE;	57	+ return FALSE;
58	+ }	58	+ }
59	+ while (vec && *vec){	59	+ while (vec && *vec){
60	+ if ( strcmp(*vec, "--") == 0 ) break;
61	+ if ( strcmp(*vec, "--rsh") == 0	60	+ if ( strcmp(*vec, "--rsh") == 0
62	+ \|\| strncmp(*vec, "--rsh=", strlen("--rsh=")) == 0 ){	61	+ \|\| strcmp(*vec, "--daemon") == 0
		62	+ \|\| strcmp(*vec, "--config") == 0
		63	+ \|\| strncmp(*vec, "--rsh=", strlen("--rsh=")) == 0
		64	+ \|\| strncmp(*vec, "--config=", strlen("--config=")) == 0 ){
63	+ regfree(&re);	65	+ regfree(&re);
64	+ return FALSE;	66	+ return FALSE;
65	+ }	67	+ }
Lines 73-79 Link Here
73	+ vec++;	75	+ vec++;
74	+ }	76	+ }
75	+ regfree(&re);	77	+ regfree(&re);
76	+ if ( e_found && !server ) return FALSE;
77	+ return TRUE;	78	+ return TRUE;
78	+}	79	+}
79	+	80	+
Lines 82-91 Link Here
82	* check_command_line() - take the command line passed to rssh, and verify	83	* check_command_line() - take the command line passed to rssh, and verify
83	* that the specified command is one the user is	84	* that the specified command is one the user is
84	* allowed to run and validate the arguments. Return the	85	* allowed to run and validate the arguments. Return the
85	@@ -230,14 +298,10 @@	86	@@ -229,16 +295,27 @@ char check_command_line( char cl, ShellOptions_t o
		87	}
86		88
87	if ( check_command(*cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){	89	if ( check_command(*cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){
88	/* filter -e option */	90	- /* filter -e option */
89	- if ( opt_filter(cl, 'e') ) return NULL;	91	- if ( opt_filter(cl, 'e') ) return NULL;
90	- while (cl && *cl){	92	- while (cl && *cl){
91	- if ( strstr(*cl, "--rsh" ) ){	93	- if ( strstr(*cl, "--rsh" ) ){
Lines 94-103 Link Here
94	- return NULL;	96	- return NULL;
95	- }	97	- }
96	- cl++;	98	- cl++;
97	+ if ( !rsync_e_okay(cl) ){	99	+ if ( !rsync_okay(cl) ){
98	+ fprintf(stderr, "\ninsecure -e or --rsh option not allowed.");	100	+ fprintf(stderr, "\ninsecure rsync options not allowed.");
99	+ log_msg("insecure -e or --rsh option in rsync command line!");	101	+ log_msg("insecure rsync options in rsync command line!");
100	+ return NULL;	102	+ return NULL;
101	}	103	}
		104	+
		105	+ /*
		106	+ * rsync is linked with popt, which recognizes a configuration
		107	+ * file ~/.popt that can, among other things, define aliases.
		108	+ * If someone can write to the home directory of the rssh
		109	+ * user, they can upload a ~/.popt file that contains
		110	+ * something like "rsync alias --server --rsh" and then
		111	+ * execute commands they upload. popt does not try to read
		112	+ * its configuration file if HOME is not set, so unset HOME to
		113	+ * disable this behavior.
		114	+ */
		115	+ if ( unsetenv("HOME") < 0 ){
		116	+ log_msg("cannot unsetenv() HOME");
		117	+ return NULL;
		118	+ }
		119	+
102	return PATH_RSYNC;	120	return PATH_RSYNC;
103	}	121	}
		122	/* No match, return NULL */

Line 0 Link Here

(-)shells/rssh/files/patch-log.c (+22 lines)
	1	Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12
	2	--- log.c.orig 2012-11-27 00:25:13 UTC
	3	+++ log.c
	4	@@ -93,10 +93,14 @@ char log_make_ident( const char name )
	5	}
	6	/* assign new value to ident from name */
	7	if ( !name ) return (ident = NULL);
	8	- ident = strdup(basename((char*)name));
	9	- /* remove leading '-' from ident, if there is one */
	10	- if ( ident[0] == '-' ){
	11	- temp = strdup(ident + 1);
	12	+ /* clone name in case basename() is POSIX-compliant */
	13	+ temp = strdup ((char *) name);
	14	+ /* always pass writeable string to basename() */
	15	+ ident = strdup (basename (temp));
	16	+ free (temp);
	17	+ /* safely remove leading '-' from ident, if there is one */
	18	+ if ((ident != NULL) && (ident[0] == '-')){
	19	+ temp = strdup(&ident[1]);
	20	free(ident);
	21	ident = temp;
	22	}

Line 0 Link Here

(-)shells/rssh/files/patch-rssh__chroot__helper.c (+29 lines)
	1	Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12
	2
	3	Incorporates also a patch to check the command line after chroot. Taken from
	4	Debian ("0010-Check-command-line-after-chroot.patch")
	5
	6	--- rssh_chroot_helper.c.orig 2006-12-21 22:22:35 UTC
	7	+++ rssh_chroot_helper.c
	8	@@ -159,7 +159,7 @@ int main( int argc, char **argv )
	9	opts.chroot_path = NULL;
	10
	11	/* figure out our name, and give it to the log module */
	12	- progname = strdup(log_make_ident(basename(argv[0])));
	13	+ progname = strdup(log_make_ident(basename(strdup (argv[0]))));
	14
	15	/* get user's passwd info */
	16	if ( (temp = getpwuid(getuid())) ){
	17	@@ -217,6 +217,12 @@ int main( int argc, char **argv )
	18	if ( !(argvec = build_arg_vector(argv[2], 0)) )
	19	ch_fatal_error("build_arg_vector()", argv[2],
	20	"bad expansion");
	21	+
	22	+ /* check the command for safety */
	23	+ if ( !check_command_line(argvec, &opts) ){
	24	+ fprintf(stderr, "\n");
	25	+ exit(1);
	26	+ }
	27
	28	/*
	29	* This is the old way to figure out what program to run. Since we're

Line 0 Link Here

(-)shells/rssh/files/patch-util.c (+105 lines)
		1	Workaround for basename(3) that is POSIX compliant since r308264 in FreeBSD 12
		2
		3	Fixes buffer allocation for the fail message. Taken from Debian
		4	("0003-Fix-buffer-allocation-buffer-for-fail-message").
		5
		6	Tightens the check for scp command line arguments that fixes also
		7	"CVE-2019-1000018". Taken from Debian ("0009-Verify-scp-command-options").
		8	Please note that with this patch the scp option "-3" can no longer be used.
		9
		10	--- util.c.orig 2012-11-27 01:14:49 UTC
		11	+++ util.c
		12	@@ -84,7 +84,7 @@ void fail( int flags, int argc, char **argv )
		13	/* create msg indicating what is allowed */
		14	if ( !size ) cmd = "This user is locked out.";
		15	else {
		16	- size += 18;
		17	+ size += 18 + 1;
		18	if ( !(cmd = (char *)malloc(size)) ){
		19	log_msg("fatal error: out of mem allocating log msg");
		20	exit(1);
		21	@@ -165,6 +165,7 @@ bool check_command( char cl, ShellOptions_t opts, ch
		22	{
		23	char prog; / basename of cmd */
		24	char *tmp = cl;
		25	+ char *tmp2 = NULL;
		26	bool need_free = FALSE;
		27	bool rc = FALSE;
		28	int i;
		29	@@ -186,11 +187,17 @@ bool check_command( char cl, ShellOptions_t opts, ch
		30	}
		31
		32	/* compare tmp to cmd and prog for match */
		33	- prog = basename(cmd);
		34	+ tmp2 = strdup (cmd);
		35	+ if (tmp2 == NULL) {
		36	+ log_msg ("strdup() failed in check_command()");
		37	+ return FALSE;
		38	+ }
		39	+ prog = basename(tmp2);
		40	if ( !(strcmp(tmp, cmd)) \|\| !(strcmp(tmp, prog))){
		41	log_msg("cmd '%s' approved", prog);
		42	rc = TRUE;
		43	}
		44	+ free (tmp2);
		45	}
		46	if (need_free) free(tmp);
		47	return rc;
		48	@@ -198,6 +205,43 @@ bool check_command( char cl, ShellOptions_t opts, ch
		49
		50
		51	/*
		52	+ * scp_okay() - take the command line and check that it is a hopefully-safe scp
		53	+ * server command line, accepting only very specific options.
		54	+ * Returns FALSE if the command line should not be allowed, TRUE
		55	+ * if it is okay.
		56	+ */
		57	+static int scp_okay( char **vec )
		58	+{
		59	+ int saw_f_or_t = FALSE;
		60	+
		61	+ for ( vec++; vec && *vec; vec++ ){
		62	+ /* Allowed options. */
		63	+ if ( strcmp(*vec, "-v") == 0 ) continue;
		64	+ if ( strcmp(*vec, "-r") == 0 ) continue;
65	+ if ( strcmp(*vec, "-p") == 0 ) continue;
66	+ if ( strcmp(*vec, "-d") == 0 ) continue;
67	+ if ( strcmp(vec, "-f") == 0 \|\| strcmp(vec, "-pf") == 0 ){
68	+ saw_f_or_t = TRUE;
69	+ continue;
70	+ }
71	+ if ( strcmp(vec, "-t") == 0 \|\| strcmp(vec, "-pt") == 0 ){
72	+ saw_f_or_t = TRUE;
73	+ continue;
74	+ }
75	+
76	+ /* End of arguments. */
77	+ if ( strcmp(*vec, "--") == 0 ) break;
78	+
79	+ /* Any other argument is not allowed. */
80	+ if ( *vec[0] == '-' ) return FALSE;
81	+ }
82	+
83	+ /* Either -f or -t must have been given. */
84	+ return saw_f_or_t;
85	+}
86	+
87	+
88	+/*
89	* check_command_line() - take the command line passed to rssh, and verify
90	* that the specified command is one the user is
91	* allowed to run and validate the arguments. Return the
92	@@ -212,8 +256,11 @@ char check_command_line( char cl, ShellOptions_t o
93	return PATH_SFTP_SERVER;
94
95	if ( check_command(*cl, opts, PATH_SCP, RSSH_ALLOW_SCP) ){
96	- /* filter -S option */
97	- if ( opt_filter(cl, 'S') ) return NULL;
98	+ if ( !scp_okay(cl) ){
99	+ fprintf(stderr, "\ninsecure scp option not allowed.");
100	+ log_msg("insecure scp option in scp command line");
101	+ return NULL;
102	+ }
103	return PATH_SCP;
104	}
105