Allow building without the exec() supported ...

.. to satisfy restricted environment or fuzzers

We are encountering weird issues in the oss-fuzz that the file disappears during
coverage build so I assume some corpus sneaked in, that contains some commands
that end up being executed as part of the coverage run causing it randomly
failing.

The solution I propose is to build fuzzers without ability to call arbitrary
commands on the filesystem (such as `rm -rf /`) as this is not the point the
fuzzers should be testing.

This is controlled by the WITH_EXEC CMake option (enabled by default).

https://github.com/google/oss-fuzz/issues/10136

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Reviewed-by: Sahana Prasad <sahana@redhat.com>
Reviewed-by: Eshan Kelkar <eshankelkar@galorithm.com>

.gitlab-ci.yml

@@ -217,6 +217,7 @@ fedora/openssl_3.0.x/x86_64/minimal:
   variables:
   script:
     - cmake $CMAKE_DEFAULT_OPTIONS
+      -DWITH_EXEC=OFF
       -DWITH_SFTP=OFF
       -DWITH_SERVER=OFF
       -DWITH_ZLIB=OFF

CMakeLists.txt

@@ -226,6 +226,7 @@ message(STATUS "Server support : ${WITH_SERVER}")
 message(STATUS "GSSAPI support : ${WITH_GSSAPI}")
 message(STATUS "GEX support : ${WITH_GEX}")
 message(STATUS "Support insecure none cipher and MAC : ${WITH_INSECURE_NONE}")
+message(STATUS "Support exec : ${WITH_EXEC}")
 message(STATUS "Pcap debugging support : ${WITH_PCAP}")
 message(STATUS "Build shared library: ${BUILD_SHARED_LIBS}")
 message(STATUS "Unit testing: ${UNIT_TESTING}")

DefineOptions.cmake

@@ -23,6 +23,7 @@ option(WITH_SYMBOL_VERSIONING "Build with symbol versioning" ON)
 option(WITH_ABI_BREAK "Allow ABI break" OFF)
 option(WITH_GEX "Enable DH Group exchange mechanisms" ON)
 option(WITH_INSECURE_NONE "Enable insecure none cipher and MAC algorithms (not suitable for production!)" OFF)
+option(WITH_EXEC "Enable libssh to execute arbitrary commands from configuration files or options (match exec, proxy commands and OpenSSH-based proxy-jumps)." ON)
 option(FUZZ_TESTING "Build with fuzzer for the server and client (automatically enables none cipher!)" OFF)
 option(PICKY_DEVELOPER "Build with picky developer flags" OFF)
 
@@ -60,3 +61,7 @@ endif (NOT GLOBAL_CLIENT_CONFIG)
 if (FUZZ_TESTING)
   set(WITH_INSECURE_NONE ON)
 endif (FUZZ_TESTING)
+
+if (WIN32)
+    set(WITH_EXEC 0)
+endif(WIN32)

config.h.cmake

@@ -231,9 +231,14 @@
 /* Define to 1 if you want to enable DH group exchange algorithms */
 #cmakedefine WITH_GEX 1
 
-/* Define to 1 if you want to enable none cipher and MAC */
+/* Define to 1 if you want to enable insecure none cipher and MAC */
 #cmakedefine WITH_INSECURE_NONE 1
 
+/* Define to 1 if you want to allow libssh to execute arbitrary commands from
+ * configuration files or options (match exec, proxy commands and OpenSSH-based
+ * proxy-jumps). */
+#cmakedefine WITH_EXEC 1
+
 /* Define to 1 if you want to enable blowfish cipher support */
 #cmakedefine WITH_BLOWFISH_CIPHER 1
 

include/libssh/socket.h

@@ -37,8 +37,8 @@ void ssh_socket_set_fd(ssh_socket s, socket_t fd);
 socket_t ssh_socket_get_fd(ssh_socket s);
 void ssh_socket_set_connected(ssh_socket s, struct ssh_poll_handle_struct *p);
 int ssh_socket_unix(ssh_socket s, const char *path);
+#if WITH_EXEC
 void ssh_execute_command(const char *command, socket_t in, socket_t out);
-#ifndef _WIN32
 int ssh_socket_connect_proxycommand(ssh_socket s, const char *command);
 #endif
 void ssh_socket_close(ssh_socket s);

src/client.c

@@ -589,11 +589,16 @@ int ssh_connect(ssh_session session)
         session->session_state = SSH_SESSION_STATE_SOCKET_CONNECTED;
         ssh_socket_set_fd(session->socket, session->opts.fd);
         ret = SSH_OK;
-#ifndef _WIN32
     } else if (session->opts.ProxyCommand != NULL) {
+#ifdef WITH_EXEC
         ret = ssh_socket_connect_proxycommand(session->socket,
                 session->opts.ProxyCommand);
-#endif
+#else
+        ssh_set_error(session,
+                      SSH_FATAL,
+                      "The libssh is built without support for proxy commands.");
+        ret = SSH_ERROR;
+#endif /* WITH_EXEC */
     } else {
         ret = ssh_socket_connect(session->socket,
                                  session->opts.host,

src/config.c

@@ -305,20 +305,8 @@ ssh_config_match(char *value, const char *pattern, bool negate)
     return result;
 }
 
-#ifdef _WIN32
-static int
-ssh_match_exec(ssh_session session, const char *command, bool negate)
-{
-    (void) session;
-    (void) command;
-    (void) negate;
-
-    SSH_LOG(SSH_LOG_TRACE, "Unsupported 'exec' command on Windows '%s'",
-            command);
-    return 0;
-}
-#else /* _WIN32 */
-
+#ifdef WITH_EXEC
+/* FIXME reuse the ssh_execute_command() from socket.c */
 static int
 ssh_exec_shell(char *cmd)
 {
@@ -431,7 +419,20 @@ ssh_match_exec(ssh_session session, const char *command, bool negate)
     free(cmd);
     return result;
 }
-#endif  /* _WIN32 */
+#else
+static int
+ssh_match_exec(ssh_session session, const char *command, bool negate)
+{
+    (void)session;
+    (void)command;
+    (void)negate;
+
+    SSH_LOG(SSH_LOG_TRACE,
+            "Unsupported 'exec' command on Windows '%s'",
+            command);
+    return 0;
+}
+#endif /* WITH_EXEC */
 
 /* @brief: Parse the ProxyJump configuration line and if parsing,
  * stores the result in the configuration option

src/socket.c

@@ -881,7 +881,7 @@ int ssh_socket_connect(ssh_socket s,
     return SSH_OK;
 }
 
-#ifndef _WIN32
+#ifdef WITH_EXEC
 /**
  * @internal
  * @brief executes a command and redirect input and outputs
@@ -910,7 +910,7 @@ ssh_execute_command(const char *command, socket_t in, socket_t out)
      */
     shell = getenv("SHELL");
     if (shell == NULL || shell[0] == '\0') {
-        /* Fall back to the /bin/sh only if the bash is not available. But there are 
+        /* Fall back to the /bin/sh only if the bash is not available. But there are
          * issues with dash or whatever people tend to link to /bin/sh */
         rc = access("/bin/bash", 0);
         if (rc != 0) {
@@ -947,7 +947,6 @@ ssh_execute_command(const char *command, socket_t in, socket_t out)
  * @returns SSH_OK socket is being connected.
  * @returns SSH_ERROR error while executing the command.
  */
-
 int
 ssh_socket_connect_proxycommand(ssh_socket s, const char *command)
 {
@@ -987,6 +986,5 @@ ssh_socket_connect_proxycommand(ssh_socket s, const char *command)
 
     return SSH_OK;
 }
-
-#endif /* _WIN32 */
+#endif /* WITH_EXEC */
 /** @} */

tests/client/torture_proxycommand.c

@@ -69,7 +69,9 @@ static void torture_options_set_proxycommand(void **state)
     char command[255] = {0};
     struct stat sb;
     int rc;
+#ifdef WITH_EXEC
     socket_t fd;
+#endif
 
     rc = stat(NCAT_EXECUTABLE, &sb);
     if (rc != 0 || (sb.st_mode & S_IXOTH) == 0) {
@@ -88,11 +90,15 @@ static void torture_options_set_proxycommand(void **state)
     rc = ssh_options_set(session, SSH_OPTIONS_PROXYCOMMAND, command);
     assert_int_equal(rc, 0);
     rc = ssh_connect(session);
+#ifdef WITH_EXEC
     assert_ssh_return_code(session, rc);
     fd = ssh_get_fd(session);
     assert_true(fd != SSH_INVALID_SOCKET);
     rc = fcntl(fd, F_GETFL);
     assert_int_equal(rc & O_RDWR, O_RDWR);
+#else
+    assert_int_equal(rc, SSH_ERROR);
+#endif /* WITH_EXEC */
 }
 
 #else /* NCAT_EXECUTABLE */
@@ -124,7 +130,9 @@ static void torture_options_set_proxycommand_ssh(void **state)
     const char *address = torture_server_address(AF_INET);
     char command[255] = {0};
     int rc;
+#ifdef WITH_EXEC
     socket_t fd;
+#endif
 
     rc = snprintf(command, sizeof(command),
                   "ssh -oStrictHostKeyChecking=no -oUserKnownHostsFile=/dev/null -W [%%h]:%%p alice@%s",
@@ -134,11 +142,15 @@ static void torture_options_set_proxycommand_ssh(void **state)
     rc = ssh_options_set(session, SSH_OPTIONS_PROXYCOMMAND, command);
     assert_int_equal(rc, 0);
     rc = ssh_connect(session);
+#ifdef WITH_EXEC
     assert_ssh_return_code(session, rc);
     fd = ssh_get_fd(session);
     assert_true(fd != SSH_INVALID_SOCKET);
     rc = fcntl(fd, F_GETFL);
     assert_int_equal(rc & O_RDWR, O_RDWR);
+#else
+    assert_int_equal(rc, SSH_ERROR);
+#endif /* WITH_EXEC */
 }
 
 static void torture_options_set_proxycommand_ssh_stderr(void **state)
@@ -148,7 +160,9 @@ static void torture_options_set_proxycommand_ssh_stderr(void **state)
     const char *address = torture_server_address(AF_INET);
     char command[255] = {0};
     int rc;
+#ifdef WITH_EXEC
     socket_t fd;
+#endif
 
     /* The -vvv switches produce the desired output on the standard error */
     rc = snprintf(command, sizeof(command),
@@ -159,11 +173,15 @@ static void torture_options_set_proxycommand_ssh_stderr(void **state)
     rc = ssh_options_set(session, SSH_OPTIONS_PROXYCOMMAND, command);
     assert_int_equal(rc, 0);
     rc = ssh_connect(session);
+#ifdef WITH_EXEC
     assert_ssh_return_code(session, rc);
     fd = ssh_get_fd(session);
     assert_true(fd != SSH_INVALID_SOCKET);
     rc = fcntl(fd, F_GETFL);
     assert_int_equal(rc & O_RDWR, O_RDWR);
+#else
+    assert_int_equal(rc, SSH_ERROR);
+#endif /* WITH_EXEC */
 }
 
 static void torture_options_proxycommand_injection(void **state)

tests/fuzz/README.md

@@ -10,6 +10,8 @@ but they are suitable for debugging.
 
 ## Background
 
+### Turn off encryption
+
 Fuzzing ssh protocol is complicated by the way that all the communication
 between client and server is encrypted and authenticated using keys based
 on random data, making it impossible to fuzz the actual underlying protocol
@@ -17,6 +19,17 @@ as every change in the encrypted data causes integrity errors. For that reason,
 libssh needs to implement "none" cipher and MAC as described in RFC 4253
 and these need to be used during fuzzing to be able to accomplish
 reproducibility and for fuzzers to be able to progress behind key exchange.
+This is enabled with the `WITH_INSECURE_NONE` CMake option.
+
+### Do not allow filesystem modification
+
+The OpenSSH configuration files are quite rich and expects users to know what
+they do when they write their configuration files. The fuzzer driver is not an
+average user so it is very happy to try whatever commands come to its "mind",
+including `rm -rf /` and libssh would be very happy to run it by default. This
+might remove some parts of the system that are mandatory for fuzzing.
+To avoid executing dangerous commands like this, the `WITH_EXEC=OFF` CMake
+option prevents invoking any external command through `exec()` syscall.
 
 ## Corpus creation
 
@@ -31,7 +44,7 @@ to use none cipher for the key exchange to be plausible.
 
  * Compile libssh with support for none cipher and pcap:
 
-    cmake -DWITH_INSECURE_NONE=ON -DWITH_PCAP=ON ../
+    cmake -DWITH_INSECURE_NONE=ON -DWITH_EXEC=OFF -DWITH_PCAP=ON ../
 
  * Create a configuration file enabling none cipher and mac:
 

tests/unittests/torture_config.c

@@ -862,7 +862,7 @@ static void torture_config_match(void **state,
     }
     torture_reset_config(session);
     _parse_config(session, file, string, SSH_OK);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_string_equal(session->opts.host, "otherhost");
 #else
@@ -878,7 +878,7 @@ static void torture_config_match(void **state,
     }
     torture_reset_config(session);
     _parse_config(session, file, string, SSH_OK);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_string_equal(session->opts.host, "otherhost");
 #else
@@ -894,7 +894,7 @@ static void torture_config_match(void **state,
     }
     torture_reset_config(session);
     _parse_config(session, file, string, SSH_OK);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_string_equal(session->opts.host, "otherhost");
 #else
@@ -1855,7 +1855,7 @@ static void torture_config_parser_get_cmd(void **state)
 {
     char *p = NULL, *tok = NULL;
     char data[256];
-#ifdef __unix__
+#ifdef WITH_EXEC
     FILE *outfile = NULL, *infile = NULL;
     int pid;
     char buffer[256] = {0};
@@ -1899,7 +1899,7 @@ static void torture_config_parser_get_cmd(void **state)
     assert_string_equal(tok, data);
     assert_int_equal(*p, '\0');
 
-#ifdef __unix__
+#ifdef WITH_EXEC
     /* Check if the command would get correctly executed
      * Use the script file "hello world.sh" to echo the first argument
      * Run as <= "/workdir/hello world.sh" "hello libssh" => */
@@ -1929,7 +1929,7 @@ static void torture_config_parser_get_cmd(void **state)
 
     fclose(outfile);
     assert_string_equal(buffer, "hello libssh");
-#endif
+#endif /* WITH_EXEC */
 }
 
 /* ssh_config_get_token() should behave as expected

tests/unittests/torture_options.c

@@ -1147,7 +1147,7 @@ static void torture_options_config_match(void **state)
 
     rv = ssh_options_parse_config(session, "test_config");
     assert_ssh_return_code(session, rv);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_int_equal(session->opts.port, 34);
 #else
@@ -1169,7 +1169,7 @@ static void torture_options_config_match(void **state)
 
     rv = ssh_options_parse_config(session, "test_config");
     assert_ssh_return_code(session, rv);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_int_equal(session->opts.port, 34);
 #else
@@ -1222,7 +1222,7 @@ static void torture_options_config_match_multi(void **state)
 
     rv = ssh_options_parse_config(session, "test_config");
     assert_ssh_return_code(session, rv);
-#ifdef _WIN32
+#ifndef WITH_EXEC
     /* The match exec is not supported on windows at this moment */
     assert_int_equal(session->opts.port, 34);
 #else