ssh: option injection code review

src/libgit2/transports/ssh_exec.c

@@ -132,26 +132,22 @@ static int get_ssh_cmdline(
 	const char *default_ssh_cmd = "ssh";
 	int error;
 
-	/* Safety check: like git, we forbid paths that look like an option as
-	 * that could lead to injection to ssh that can make us do unexpected
-	 * things */
-	if (git_net_looks_like_command_line_option(url->username)) {
-		git_error_set(GIT_ERROR_NET, "strange username '%s' blocked", url->username);
+	/*
+	 * Safety check: like git, we forbid paths that look like an
+	 * option as that could lead to injection to ssh that can make
+	 * us do unexpected things
+	 */
+	if (git_process__is_cmdline_option(url->username)) {
+		git_error_set(GIT_ERROR_NET, "cannot ssh: username '%s' is ambiguous with command-line option", url->username);
 		return -1;
-	}
-	if (git_net_looks_like_command_line_option(url->host)) {
-		git_error_set(GIT_ERROR_NET, "strange host '%s' blocked", url->host);
+	} else if (git_process__is_cmdline_option(url->host)) {
+		git_error_set(GIT_ERROR_NET, "cannot ssh: host '%s' is ambiguous with command-line option", url->host);
 		return -1;
-	}
-
-	/* Safety check: like git, we forbid paths that look like an option as
-	 * that could lead to injection on the remote side */
-	if (git_net_looks_like_command_line_option(url->path)) {
-		git_error_set(GIT_ERROR_NET, "strange path '%s' blocked", url->path);
+	} else if (git_process__is_cmdline_option(url->path)) {
+		git_error_set(GIT_ERROR_NET, "cannot ssh: path '%s' is ambiguous with command-line option", url->path);
 		return -1;
 	}
 
-
 	if ((error = git_repository_config_snapshot(&cfg, repo)) < 0)
 		return error;
 

src/libgit2/transports/ssh_libssh2.c

@@ -14,6 +14,7 @@
 #include "runtime.h"
 #include "net.h"
 #include "smart.h"
+#include "process.h"
 #include "streams/socket.h"
 #include "sysdir.h"
 
@@ -790,8 +791,8 @@ static int _git_ssh_setup_conn(
 
 	/* Safety check: like git, we forbid paths that look like an option as
 	 * that could lead to injection on the remote side */
-	if (git_net_looks_like_command_line_option(s->url.path)) {
-		git_error_set(GIT_ERROR_NET, "strange path '%s' blocked", s->url.path);
+	if (git_process__is_cmdline_option(s->url.path)) {
+		git_error_set(GIT_ERROR_NET, "cannot ssh: path '%s' is ambiguous with command-line option", s->url.path);
 		error = -1;
 		goto done;
 	}

src/util/net.c

@@ -1152,8 +1152,3 @@ void git_net_url_dispose(git_net_url *url)
 	git__free(url->username); url->username = NULL;
 	git__free(url->password); url->password = NULL;
 }
-
-int git_net_looks_like_command_line_option(const char *str)
-{
-	return str && str[0] == '-';
-}

src/util/net.h

@@ -107,13 +107,4 @@ extern bool git_net_url_matches_pattern_list(
 /** Disposes the contents of the structure. */
 extern void git_net_url_dispose(git_net_url *url);
 
-
-/**
- * Returns true if the string starts with a dash
- *
- * These could be used to try to trick an executed subcommand like ssh to do
- * something other than what we intend.
- */
-int git_net_looks_like_command_line_option(const char *str);
-
 #endif

src/util/process.h

@@ -106,6 +106,17 @@ extern int git_process__cmdline(
 
 #endif
 
+/*
+ * Whether the given string looks like a command line option (starts
+ * with a dash). This is useful for examining strings that will become
+ * cmdline arguments to ensure that they are not erroneously treated
+ * as an option. For example, arguments to `ssh`.
+ */
+GIT_INLINE(bool) git_process__is_cmdline_option(const char *str)
+{
+	return (str && str[0] == '-');
+}
+
 /**
  * Start the process.
  *