From owner-svn-ports-branches@freebsd.org Thu Mar 7 14:59:38 2019 Return-Path: Delivered-To: svn-ports-branches@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5EEEA1531D25; Thu, 7 Mar 2019 14:59:38 +0000 (UTC) (envelope-from kai@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 038A58982B; Thu, 7 Mar 2019 14:59:38 +0000 (UTC) (envelope-from kai@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id D216B1D3F3; Thu, 7 Mar 2019 14:59:37 +0000 (UTC) (envelope-from kai@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x27ExbOX068596; Thu, 7 Mar 2019 14:59:37 GMT (envelope-from kai@FreeBSD.org) Received: (from kai@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x27ExaDv068591; Thu, 7 Mar 2019 14:59:36 GMT (envelope-from kai@FreeBSD.org) Message-Id: <201903071459.x27ExaDv068591@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kai set sender to kai@FreeBSD.org using -f From: Kai Knoblich Date: Thu, 7 Mar 2019 14:59:36 +0000 (UTC) To: ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-branches@freebsd.org Subject: svn commit: r494951 - in branches/2019Q1/shells/rssh: . files X-SVN-Group: ports-branches X-SVN-Commit-Author: kai X-SVN-Commit-Paths: in branches/2019Q1/shells/rssh: . files X-SVN-Commit-Revision: 494951 X-SVN-Commit-Repository: ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 038A58982B X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.93 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.93)[-0.933,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-ports-branches@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for all the branches of the ports tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2019 14:59:38 -0000 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@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 @@ -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; + } +