Arbitrary code execution on clients through malicious svn+ssh URLs in svn:externals and svn:sync-from-url Summary: ======== A Subversion client sometimes connects to URLs provided by the repository. This happens in two primary cases: during 'checkout', 'export', 'update', and 'switch', when the tree being downloaded contains svn:externals properties; and when using 'svnsync sync' with one URL argument. A maliciously constructed svn+ssh:// URL would cause Subversion clients to run an arbitrary shell command. Such a URL could be generated by a malicious server, by a malicious user committing to a honest server (to attack another user of that server's repositories), or by a proxy server. The vulnerability affects all clients, including those that use file://, http://, and plain (untunneled) svn://. An exploit has been tested. Known vulnerable: ================= Subversion clients 1.0.0 through 1.8.18 (inclusive) Subversion clients 1.9.0 through 1.9.6 (inclusive) Subversion client 1.10.0-alpha3 Subversion 1.10.0-alpha1 and 1.10.0-alpha2 are vulnerable, however, were never publicly released. Known fixed: ============ Subversion 1.8.19 Subversion 1.9.7 Patches are available for 1.9, 1.8, 1.6. The patch for 1.9 applies to 1.10.0-alpha3 with an offset. The patch for 1.8 applies to 1.7 with an offset. Clients that do not have access to an ssh client, and have no custom tunnels configured in their runtime configuration area [1], are not vulnerable. Clients using Subversion's own runtime module loading for Repository Access (RA) modules are not vulnerable if the 'libsvn_ra_svn' module, which provides support for the svn+ssh:// and svn:// protocols is removed. [1] http://svnbook.red-bean.com/en/1.7/svn.advanced.confarea.html#svn.advanced.confarea.layout This link describes Subversion 1.7, but the description is correct for all other versions as well. Details: ======== OpenSSH implements a ProxyCommand feature which instructs the client to run an additional local command before opening a connection to the server. The intention is that this local command will run a proxy server for the connection to the SSH server. This feature can be enabled on the command line with the -oProxyCommand option switch. The -oProxyCommand option takes an arbitrary command as its argument which will be executed by the ssh client before connecting to the SSH server. The attack makes use of this feature by placing a ProxyCommand option with an arbitrary command into an svn+ssh:// URL. A vulnerable svn client will pass this option to the ssh command, which in turn will execute the arbitrary command provided by the attacker. PuTTY's plink SSH client implements the same feature with a slightly different option name "-proxycmd". Severity: ========= CVSSv3 Base Score: 9.9 (Critical) CVSSv3 Base Vector: CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:H/E:H/RL:O/RC:C A successful exploit will run an arbitrary shell command with the privileges of the Subversion client. Recommendations: ================ Several alternative ways to fix the issue are available. Any one of #1, #2, #3, and #4 fixes the issue completely. There is no need to implement more than one of these four options. 1. We recommend all users to upgrade to Subversion 1.9.7 or 1.8.19. New Subversion source and binary packages can be found at: https://subversion.apache.org/download https://subversion.apache.org/packages 2. Users of Subversion 1.8.x and 1.9.x who are unable to upgrade may apply the included patch. 3. Clients that are not able to execute the 'ssh' command are not vulnerable. The name of the ssh command which is executed may be changed by setting the $SVN_SSH environment variable or by setting a value for the 'ssh' key in the "[tunnels]" section of the file "config" in the runtime configuration area [1]. By default, only the "ssh" tunnel is configured. It is available even if it is commented out in the file "config". The default definition of the "ssh" tunnel is equivalent to: ssh = $SVN_SSH ssh -q If the value of this option is set to a non-existent path, then svn+ssh:// URLs will no longer work but the svn client will not be vulnerable: ssh = /this/path/does/not/exist However, if OpenSSH is used as SSH client (the default on most UNIX-like systems), then svn+ssh:// URLs can still be used safely. Either change the configuration file setting to: ssh = $SVN_SSH ssh -q -- or set the environment variable "SVN_SSH" to the value: ssh -q -- The -- argument tells OpenSSH to stop processing subsequent arguments as command line options, and hence neuters the attack because a -oProxyCommand option on the command line will no longer be evaluated as an option. If PuTTY is used as SSH client (the default on Windows systems, including TortoiseSVN) this trick WILL NOT WORK because PuTTY evaluates command line options even after -- occurs on its command line. If using PuTTY, either the svn client must be upgraded or svn+ssh:// URLs must be disabled entirely as described above. The "[tunnels]" section may define additional third-party custom tunnels; those may be vulnerable if they do not perform input validation on their first argument, which contains the hostname to connect to. Custom tunnels are invoked with three arguments: the hostname to connect to, the string "svnserve" and the string "-t". It is recommended that custom tunnel definitions be audited for correct handling of unusual or invalid host values; the Subversion libraries perform some basic validation, but cannot guarantee correct quoting/escaping of the parameters to arbitrary third-party tunnel commands. 4. Clients built to use Subversion's own runtime mechanism for loading modules can remove the libsvn_ra_svn shared module and thus remove the threat. The svn:// and svn+ssh:// protocols will no longer be available. This does not apply to clients built to use the normal compile-time linking of shared libraries: those clients will fail to start if the libsvn_ra_svn shared module is removed. Subversion's own runtime loading mechanism is enabled at build-time by using --enable-runtime-module-search. 5. Users of 'svnsync sync' should use the two-URL-arguments form of the command. The current remote URL may be found by either of these two commands: svnsync info -- $DEST_URL svn proplist --verbose --revision=0 -- $DEST_URL where $DEST_URL is the (first, or only) URL argument to 'svnsync sync'. Then, change the svnsync invocation to always pass that URL as an additional argument: change svnsync sync URL/to/sync to svnsync sync URL/to/sync URL/to/source NOTE: This recommendation applies only to 'svnsync sync' and does not fix the 'checkout' / 'update' part of the issue. 6. Server administrators may wish to install a 'pre-commit' hook that rejects commits that add invalid svn+*:// URLs, in order to protect their users from other (malicious) users committing such URLs. An example hook is available at: 7. API consumers that implement a 'svn_ra_open_tunnel_func_t open_tunnel_func' callback should audit it for issues similar to this one. 8. Subversion 1.7 and 1.6 are officially no longer supported. The patch for 1.8 will apply to 1.7 and a patch for 1.6 is available, the patch for 1.6 could be adapted to even older versions. Since 1.7 and 1.6 are no longer supported there will be no official releases of those branches for this vulnerablity. [1] http://svnbook.red-bean.com/en/1.7/svn.advanced.confarea.html#svn.advanced.confarea.layout This link describes Subversion 1.7, but the description is correct for all other versions as well. References: =========== CVE-2017-9800 (Subversion) CVE-2017-12426 (GitLab) CVE-2017-1000116 (Mercurial (hg)) CVE-2017-1000117 (Git) Reported by: ============ Jonathan Nieder Discovered by Joern Schneeweisz of Recurity Labs. Patches: ======== Patch for Subversion 1.9.6: [[[ Index: subversion/libsvn_ra_svn/client.c =================================================================== --- subversion/libsvn_ra_svn/client.c (revision 1803926) +++ subversion/libsvn_ra_svn/client.c (working copy) @@ -46,6 +46,7 @@ #include "svn_props.h" #include "svn_mergeinfo.h" #include "svn_version.h" +#include "svn_ctype.h" #include "svn_private_config.h" @@ -396,7 +397,7 @@ * versions have it too. If the user is using some other ssh * implementation that doesn't accept it, they can override it * in the [tunnels] section of the config. */ - val = "$SVN_SSH ssh -q"; + val = "$SVN_SSH ssh -q --"; } if (!val || !*val) @@ -441,7 +442,7 @@ for (n = 0; cmd_argv[n] != NULL; n++) argv[n] = cmd_argv[n]; - argv[n++] = svn_path_uri_decode(hostinfo, pool); + argv[n++] = hostinfo; argv[n++] = "svnserve"; argv[n++] = "-t"; argv[n] = NULL; @@ -802,7 +803,33 @@ } +/* A simple whitelist to ensure the following are valid: + * user@server + * [::1]:22 + * server-name + * server_name + * 127.0.0.1 + * with an extra restriction that a leading '-' is invalid. + */ +static svn_boolean_t +is_valid_hostinfo(const char *hostinfo) +{ + const char *p = hostinfo; + if (p[0] == '-') + return FALSE; + + while (*p) + { + if (!svn_ctype_isalnum(*p) && !strchr(":.-_[]@", *p)) + return FALSE; + + ++p; + } + + return TRUE; +} + static svn_error_t *ra_svn_open(svn_ra_session_t *session, const char **corrected_url, const char *url, @@ -835,8 +862,18 @@ || (callbacks->check_tunnel_func && callbacks->open_tunnel_func && !callbacks->check_tunnel_func(callbacks->tunnel_baton, tunnel)))) - SVN_ERR(find_tunnel_agent(tunnel, uri.hostinfo, &tunnel_argv, config, - result_pool)); + { + const char *decoded_hostinfo; + + decoded_hostinfo = svn_path_uri_decode(uri.hostinfo, result_pool); + + if (!is_valid_hostinfo(decoded_hostinfo)) + return svn_error_createf(SVN_ERR_BAD_URL, NULL, _("Invalid host '%s'"), + uri.hostinfo); + + SVN_ERR(find_tunnel_agent(tunnel, decoded_hostinfo, &tunnel_argv, + config, result_pool)); + } else tunnel_argv = NULL; Index: subversion/libsvn_subr/config_file.c =================================================================== --- subversion/libsvn_subr/config_file.c (revision 1803926) +++ subversion/libsvn_subr/config_file.c (working copy) @@ -1248,12 +1248,12 @@ "### passed to the tunnel agent as @ .) If the" NL "### built-in ssh scheme were not predefined, it could be defined" NL "### as:" NL - "# ssh = $SVN_SSH ssh -q" NL + "# ssh = $SVN_SSH ssh -q --" NL "### If you wanted to define a new 'rsh' scheme, to be used with" NL "### 'svn+rsh:' URLs, you could do so as follows:" NL - "# rsh = rsh" NL + "# rsh = rsh --" NL "### Or, if you wanted to specify a full path and arguments:" NL - "# rsh = /path/to/rsh -l myusername" NL + "# rsh = /path/to/rsh -l myusername --" NL "### On Windows, if you are specifying a full path to a command," NL "### use a forward slash (/) or a paired backslash (\\\\) as the" NL "### path separator. A single backslash will be treated as an" NL ]]] Patch for Subversion 1.8.18 [[[ Index: subversion/libsvn_ra_svn/client.c =================================================================== --- subversion/libsvn_ra_svn/client.c (revision 1803926) +++ subversion/libsvn_ra_svn/client.c (working copy) @@ -46,6 +46,7 @@ #include "svn_props.h" #include "svn_mergeinfo.h" #include "svn_version.h" +#include "svn_ctype.h" #include "svn_private_config.h" @@ -395,7 +396,7 @@ * versions have it too. If the user is using some other ssh * implementation that doesn't accept it, they can override it * in the [tunnels] section of the config. */ - val = "$SVN_SSH ssh -q"; + val = "$SVN_SSH ssh -q --"; } if (!val || !*val) @@ -435,7 +436,7 @@ ; *argv = apr_palloc(pool, (n + 4) * sizeof(char *)); memcpy((void *) *argv, cmd_argv, n * sizeof(char *)); - (*argv)[n++] = svn_path_uri_decode(hostinfo, pool); + (*argv)[n++] = hostinfo; (*argv)[n++] = "svnserve"; (*argv)[n++] = "-t"; (*argv)[n] = NULL; @@ -716,7 +717,33 @@ } +/* A simple whitelist to ensure the following are valid: + * user@server + * [::1]:22 + * server-name + * server_name + * 127.0.0.1 + * with an extra restriction that a leading '-' is invalid. + */ +static svn_boolean_t +is_valid_hostinfo(const char *hostinfo) +{ + const char *p = hostinfo; + if (p[0] == '-') + return FALSE; + + while (*p) + { + if (!svn_ctype_isalnum(*p) && !strchr(":.-_[]@", *p)) + return FALSE; + + ++p; + } + + return TRUE; +} + static svn_error_t *ra_svn_open(svn_ra_session_t *session, const char **corrected_url, const char *url, @@ -740,8 +767,17 @@ parse_tunnel(url, &tunnel, pool); if (tunnel) - SVN_ERR(find_tunnel_agent(tunnel, uri.hostinfo, &tunnel_argv, config, - pool)); + { + const char *decoded_hostinfo; + + decoded_hostinfo = svn_path_uri_decode(uri.hostinfo, pool); + if (!is_valid_hostinfo(decoded_hostinfo)) + return svn_error_createf(SVN_ERR_BAD_URL, NULL, _("Invalid host '%s'"), + uri.hostinfo); + + SVN_ERR(find_tunnel_agent(tunnel, decoded_hostinfo, &tunnel_argv, + config, pool)); + } else tunnel_argv = NULL; Index: subversion/libsvn_subr/config_file.c =================================================================== --- subversion/libsvn_subr/config_file.c (revision 1803926) +++ subversion/libsvn_subr/config_file.c (working copy) @@ -1134,12 +1134,12 @@ "### passed to the tunnel agent as @ .) If the" NL "### built-in ssh scheme were not predefined, it could be defined" NL "### as:" NL - "# ssh = $SVN_SSH ssh -q" NL + "# ssh = $SVN_SSH ssh -q --" NL "### If you wanted to define a new 'rsh' scheme, to be used with" NL "### 'svn+rsh:' URLs, you could do so as follows:" NL - "# rsh = rsh" NL + "# rsh = rsh --" NL "### Or, if you wanted to specify a full path and arguments:" NL - "# rsh = /path/to/rsh -l myusername" NL + "# rsh = /path/to/rsh -l myusername --" NL "### On Windows, if you are specifying a full path to a command," NL "### use a forward slash (/) or a paired backslash (\\\\) as the" NL "### path separator. A single backslash will be treated as an" NL ]]]