svn commit: r494951 - in branches/2019Q1/shells/rssh: . files
Kai Knoblich
kai at FreeBSD.org
Thu Mar 7 14:59:38 UTC 2019
Author: kai
Date: Thu Mar 7 14:59:36 2019
New Revision: 494951
URL: https://svnweb.freebsd.org/changeset/ports/494951
Log:
MFH: r494837
shells/rssh: Apply fixes for basename(3) handling and some security issues
basename(3) has been changed to be POSIX compliant in r308264. This implies
that it can possibly write to the passed string. shells/rssh passes a const
string, so it always crashes on invocation with FreeBSD 12 and later. The
new patches remedy this issue. [1] [2]
During further tests and research came to light that there were also
recently discovered security issues with the parsing of rsync/scp command
line arguments and insufficient sanitization of environment variables when
using rysnc.
The corresponding fixes have been incorporated to the new patches and the
already existing patch for the RSYNC option has been tightened for the
argument parsing. Please note that with this patch the scp option "-3" can
no longer be used. [3]
Furthermore, another patch was applied to make this port a bit more secure.
That patch handles a buffer allocation issue for an error message. [4]
PR: 235121
Submitted by: topical at gmx.net (first version) [1], Jason Harris (maintainer) [2]
Approved by: tcberner (mentor)
Obtained from: Debian [3] [4]
Security: d193aa9f-3f8c-11e9-9a24-6805ca0b38e8
Differential Revision: https://reviews.freebsd.org/D19474
Approved by: ports-secteam (riggs), mentors implicit
Added:
branches/2019Q1/shells/rssh/files/patch-log.c
- copied unchanged from r494837, head/shells/rssh/files/patch-log.c
branches/2019Q1/shells/rssh/files/patch-rssh__chroot__helper.c
- copied unchanged from r494837, head/shells/rssh/files/patch-rssh__chroot__helper.c
branches/2019Q1/shells/rssh/files/patch-util.c
- copied unchanged from r494837, head/shells/rssh/files/patch-util.c
Modified:
branches/2019Q1/shells/rssh/Makefile
branches/2019Q1/shells/rssh/files/optional-patch-util.c
Directory Properties:
branches/2019Q1/ (props changed)
Modified: branches/2019Q1/shells/rssh/Makefile
==============================================================================
--- branches/2019Q1/shells/rssh/Makefile Thu Mar 7 14:29:56 2019 (r494950)
+++ branches/2019Q1/shells/rssh/Makefile Thu Mar 7 14:59:36 2019 (r494951)
@@ -3,7 +3,7 @@
PORTNAME= rssh
PORTVERSION= 2.3.4
-PORTREVISION= 1
+PORTREVISION= 2
CATEGORIES= shells security
MASTER_SITES= SF
Modified: branches/2019Q1/shells/rssh/files/optional-patch-util.c
==============================================================================
--- branches/2019Q1/shells/rssh/files/optional-patch-util.c Thu Mar 7 14:29:56 2019 (r494950)
+++ branches/2019Q1/shells/rssh/files/optional-patch-util.c Thu Mar 7 14:59:36 2019 (r494951)
@@ -1,5 +1,8 @@
---- util.c.orig 2012-11-27 12:14:49.000000000 +1100
-+++ util.c 2013-01-09 17:52:54.000000000 +1100
+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>
@@ -8,18 +11,16 @@
/* LOCAL INCLUDES */
#include "pathnames.h"
-@@ -198,6 +199,73 @@
+@@ -198,6 +199,71 @@ bool check_command( char *cl, ShellOptions_t *opts, ch
/*
-+ * 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.
++ * 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_e_okay( char **vec )
++static int rsync_okay( char **vec )
+{
+ regex_t re;
+ int server = FALSE;
@@ -48,18 +49,19 @@
+ * 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;
-+ }
++ 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
-+ || strncmp(*vec, "--rsh=", strlen("--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;
+ }
@@ -73,7 +75,6 @@
+ vec++;
+ }
+ regfree(&re);
-+ if ( e_found && !server ) return FALSE;
+ return TRUE;
+}
+
@@ -82,10 +83,11 @@
* 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
-@@ -230,14 +298,10 @@
+@@ -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 */
+- /* filter -e option */
- if ( opt_filter(cl, 'e') ) return NULL;
- while (cl && *cl){
- if ( strstr(*cl, "--rsh" ) ){
@@ -94,10 +96,27 @@
- return NULL;
- }
- cl++;
-+ if ( !rsync_e_okay(cl) ){
-+ fprintf(stderr, "\ninsecure -e or --rsh option not allowed.");
-+ log_msg("insecure -e or --rsh option in rsync command line!");
++ 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 */
Copied: branches/2019Q1/shells/rssh/files/patch-log.c (from r494837, head/shells/rssh/files/patch-log.c)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2019Q1/shells/rssh/files/patch-log.c Thu Mar 7 14:59:36 2019 (r494951, copy of r494837, head/shells/rssh/files/patch-log.c)
@@ -0,0 +1,22 @@
+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;
+ }
Copied: branches/2019Q1/shells/rssh/files/patch-rssh__chroot__helper.c (from r494837, head/shells/rssh/files/patch-rssh__chroot__helper.c)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2019Q1/shells/rssh/files/patch-rssh__chroot__helper.c Thu Mar 7 14:59:36 2019 (r494951, copy of r494837, head/shells/rssh/files/patch-rssh__chroot__helper.c)
@@ -0,0 +1,29 @@
+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
Copied: branches/2019Q1/shells/rssh/files/patch-util.c (from r494837, head/shells/rssh/files/patch-util.c)
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ branches/2019Q1/shells/rssh/files/patch-util.c Thu Mar 7 14:59:36 2019 (r494951, copy of r494837, head/shells/rssh/files/patch-util.c)
@@ -0,0 +1,105 @@
+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;
+ }
+
More information about the svn-ports-all
mailing list