Fix: initialize ipc data structure (#1245)

* Fix: initialize ipc data structure with IPC_DT_ERROR type Initialize the data structure before parsing the json object. Now if the parse fails, the data object can be destroyed Also, pass the data structure pointer as reference. * Add: unittests for IPC openvas.
greenbone · Dec 1, 2022 · cf2f067 · cf2f067
1 parent 12f4043
commit cf2f067
Show file tree

Hide file tree

Showing 8 changed files with 205 additions and 21 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -204,7 +204,7 @@ configure_file (src/openvas_log_conf.cmake_in src/openvas_log.conf)
 enable_testing ()
 
 add_custom_target (tests
-                   DEPENDS attack-test pcap-test)
+                   DEPENDS attack-test pcap-test ipc-openvas-test)
 
 ## Program
 

diff --git a/misc/CMakeLists.txt b/misc/CMakeLists.txt
@@ -141,11 +141,18 @@ install (DIRECTORY
 
 enable_testing ()
 
+set (LINK_LIBS_FOR_TESTS cgreen
+                       ${LIBGVM_BASE_LDFLAGS}
+                       ${GLIB_LDFLAGS}
+                       ${PCAP_LDFLAGS}
+                       ${LINKER_HARDENING_FLAGS} ${CMAKE_THREAD_LIBS_INIT}
+                       ${ALIVEDETECTION_TEST_LINKER_WRAP_OPTIONS})
+
 set (ALIVEDETECTION_TEST_LINKER_WRAP_OPTIONS
-     "-Wl,-wrap,socket,-wrap,setsockopt")
-add_executable (pcap-test
-                EXCLUDE_FROM_ALL
-                pcap_tests.c)
+  "-Wl,-wrap,socket,-wrap,setsockopt")
+
+# pcap-tests
+add_executable (pcap-test EXCLUDE_FROM_ALL pcap_tests.c)
 add_test (pcap-test pcap-test)
 target_include_directories (pcap-test PRIVATE ${CGREEN_INCLUDE_DIRS})
 target_link_libraries (pcap-test cgreen
@@ -157,4 +164,16 @@ target_link_libraries (pcap-test cgreen
 add_custom_target (tests-pcap
                   DEPENDS pcap-test)
 
+# ipc-openvas-tests
+add_executable (ipc-openvas-test EXCLUDE_FROM_ALL ipc_openvas_tests.c)
+add_test (ipc-openvas-test ipc-openvas-test)
+target_include_directories (ipc-openvas-test PRIVATE ${CGREEN_INCLUDE_DIRS})
+target_link_libraries (ipc-openvas-test cgreen
+                       ${GLIB_LDFLAGS}
+                       ${GLIB_JSON_LDFLAGS}
+                       ${LINKER_HARDENING_FLAGS})
+
+add_custom_target (tests-ipc-openvas
+                  DEPENDS ipc-openvas-test)
+
 ## End
diff --git a/misc/ipc_openvas.c b/misc/ipc_openvas.c
@@ -235,22 +235,23 @@ ipc_user_agent_destroy (ipc_user_agent_t *data)
  *
  */
 void
-ipc_data_destroy (struct ipc_data *data)
+ipc_data_destroy (ipc_data_t **data)
 {
-  if (data == NULL)
+  if (*data == NULL)
     return;
-  switch (data->type)
+  switch ((*data)->type)
     {
     case IPC_DT_HOSTNAME:
-      ipc_hostname_destroy (data->ipc_hostname);
+      ipc_hostname_destroy ((*data)->ipc_hostname);
       break;
     case IPC_DT_USER_AGENT:
-      ipc_user_agent_destroy (data->ipc_user_agent);
+      ipc_user_agent_destroy ((*data)->ipc_user_agent);
       break;
     case IPC_DT_ERROR:
-      return;
+      break;
     }
-  g_free (data);
+  g_free (*data);
+  *data = NULL;
 }
 
 /**
@@ -335,15 +336,18 @@ ipc_data_from_json (const char *json, size_t len)
   JsonReader *reader = NULL;
 
   GError *err = NULL;
-  struct ipc_data *ret = NULL;
+  ipc_data_t *ret = NULL;
   ipc_user_agent_t *ua;
   ipc_hostname_t *hn;
-
   enum ipc_data_type type = IPC_DT_ERROR;
 
   if ((ret = calloc (1, sizeof (*ret))) == NULL)
     goto cleanup;
 
+  /* Initialize the type with error.
+   * Usefull for cleanup, in case of parser error. */
+  ret->type = type;
+
   parser = json_parser_new ();
   if (!json_parser_load_from_data (parser, json, len, &err))
     {
@@ -358,9 +362,9 @@ ipc_data_from_json (const char *json, size_t len)
     }
 
   type = json_reader_get_int_value (reader);
+  ret->type = type;
   json_reader_end_member (reader);
 
-  ret->type = type;
   switch (type)
     {
     case IPC_DT_ERROR:
@@ -407,13 +411,15 @@ ipc_data_from_json (const char *json, size_t len)
   if (reader)
     g_object_unref (reader);
   g_object_unref (parser);
+
   if (err != NULL)
     {
       g_warning ("%s: Unable to parse json (%s). Reason: %s", __func__, json,
                  err->message);
+
+      if (ret != NULL)
+        ipc_data_destroy (&ret);
     }
-  if (ret == NULL)
-    ipc_data_destroy (ret);
 
   return ret;
 }
diff --git a/misc/ipc_openvas.h b/misc/ipc_openvas.h
@@ -37,7 +37,7 @@ ipc_data_t *
 ipc_data_type_from_user_agent (const char *user_agent, size_t user_agent_len);
 
 void
-ipc_data_destroy (ipc_data_t *data);
+ipc_data_destroy (ipc_data_t **data);
 
 const char *
 ipc_data_to_json (ipc_data_t *data);

diff --git a/misc/ipc_openvas_tests.c b/misc/ipc_openvas_tests.c
@@ -0,0 +1,159 @@
+/* Copyright (C) 2019-2022 Greenbone Networks GmbH
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "ipc_openvas.c"
+
+#include <cgreen/cgreen.h>
+#include <cgreen/mocks.h>
+
+Describe (ipc_openvas);
+BeforeEach (ipc_openvas)
+{
+}
+AfterEach (ipc_openvas)
+{
+}
+
+Ensure (ipc_openvas, ipc_data_from_json_ua_ok)
+{
+  ipc_data_t *data_s = NULL;
+  ipc_data_t *data_r = NULL;
+  gchar *ua = "localhost";
+
+  // Preapre data to be sent
+  data_s = g_malloc0 (sizeof (ipc_data_t *));
+  data_s = ipc_data_type_from_user_agent (ua, strlen (ua));
+
+  const char *json = ipc_data_to_json (data_s);
+  ipc_data_destroy (&data_s);
+  assert_that (data_s, is_null);
+
+  // Read received data
+  data_r = g_malloc0 (sizeof (ipc_data_t));
+  data_r = ipc_data_from_json (json, strlen (json));
+  assert_that (ipc_get_user_agent_from_data (data_r),
+               is_equal_to_string ("localhost"));
+
+  ipc_data_destroy (&data_r);
+  assert_that (data_s, is_null);
+}
+
+Ensure (ipc_openvas, ipc_data_from_json_hostname_ok)
+{
+  ipc_data_t *data_s = NULL;
+  ipc_data_t *data_r = NULL;
+  gchar *hn = "localhost";
+  gchar *hns = "TLS certificate";
+
+  // Preapre data to be sent
+  data_s = g_malloc0 (sizeof (ipc_data_t *));
+  data_s = ipc_data_type_from_hostname (hns, strlen (hns), hn, strlen (hn));
+
+  const char *json = ipc_data_to_json (data_s);
+  ipc_data_destroy (&data_s);
+  assert_that (data_s, is_null);
+
+  // Read received data
+  data_r = g_malloc0 (sizeof (ipc_data_t));
+  data_r = ipc_data_from_json (json, strlen (json));
+  assert_that (ipc_get_hostname_from_data (data_r),
+               is_equal_to_string ("localhost"));
+  assert_that (ipc_get_hostname_source_from_data (data_r),
+               is_equal_to_string ("TLS certificate"));
+
+  ipc_data_destroy (&data_r);
+  assert_that (data_r, is_null);
+}
+
+Ensure (ipc_openvas, ipc_data_from_json_parse_error)
+{
+  ipc_data_t *data_r = NULL;
+  char *json_fake = NULL;
+
+  // malformed json string
+  json_fake = g_strdup (
+    "{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] (X11, U; Greenbone OS "
+    "22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] (X11, U; "
+    "Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] "
+    "(X11, U; Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 "
+    "[en] (X11, U; Greenbone OS "
+    "22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] (X11, U; "
+    "Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] "
+    "(X11, U; Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 "
+    "[en] (X11, U; Greenbone OS "
+    "22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] (X11, U; "
+    "Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] "
+    "(X11, U; Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 "
+    "[en] (X11, U; Greenbone OS "
+    "22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] (X11, U; "
+    "Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 [en] "
+    "(X11, U; Greenbone OS 22.04.4)\"}{\"type\":1,\"user-agent\":\"Mozilla/5.0 "
+    "[en] (X11, U; Greenbone OS 22.04.4)\"}{\"type\":");
+
+  // Read received data
+  data_r = g_malloc0 (sizeof (ipc_data_t *));
+  data_r = ipc_data_from_json (json_fake, strlen (json_fake));
+  assert_that (ipc_get_hostname_from_data (data_r), is_null);
+  assert_that (ipc_get_hostname_source_from_data (data_r), is_null);
+  assert_that (data_r, is_null);
+}
+
+Ensure (ipc_openvas, ipc_data_from_json_parse_many_objects)
+{
+  ipc_data_t *data_r = NULL;
+  char *json_fake = NULL;
+
+  // malformed json string
+  json_fake =
+    g_strdup ("{\"type\":0,\"source\":\"TLS "
+              "certificate\",\"hostname\":\"localhost\"}{\"type\":1,\"user-"
+              "agent\":\"Mozilla/5.0 [en] (X11, U; Greenbone OS "
+              "22.04.4)\"}");
+
+  // Read received data
+  data_r = g_malloc0 (sizeof (ipc_data_t *));
+  data_r = ipc_data_from_json (json_fake, strlen (json_fake));
+
+  assert_that (ipc_get_hostname_from_data (data_r),
+               is_equal_to_string ("localhost"));
+  assert_that (ipc_get_hostname_source_from_data (data_r),
+               is_equal_to_string ("TLS certificate"));
+
+  ipc_data_destroy (&data_r);
+  assert_that (data_r, is_null);
+}
+
+int
+main (int argc, char **argv)
+{
+  TestSuite *suite;
+
+  suite = create_test_suite ();
+
+  add_test_with_context (suite, ipc_openvas, ipc_data_from_json_ua_ok);
+  add_test_with_context (suite, ipc_openvas, ipc_data_from_json_hostname_ok);
+  add_test_with_context (suite, ipc_openvas, ipc_data_from_json_parse_error);
+  add_test_with_context (suite, ipc_openvas,
+                         ipc_data_from_json_parse_many_objects);
+
+  if (argc > 1)
+    return run_single_test (suite, argv[1], create_text_reporter ());
+
+  return run_test_suite (suite, create_text_reporter ());
+}
diff --git a/misc/user_agent.c b/misc/user_agent.c
@@ -44,7 +44,7 @@ send_user_agent_via_ipc (struct ipc_context *ipc_context)
 
   ua = ipc_data_type_from_user_agent (user_agent, strlen (user_agent));
   json = ipc_data_to_json (ua);
-  ipc_data_destroy (ua);
+  ipc_data_destroy (&ua);
   if (ipc_send (ipc_context, IPC_MAIN, json, strlen (json)) < 0)
     g_warning ("Unable to send %s to host process", user_agent);
 }

diff --git a/nasl/nasl_host.c b/nasl/nasl_host.c
@@ -140,7 +140,7 @@ add_hostname (lex_ctxt *lexic)
   hn = ipc_data_type_from_hostname (source, strlen (source), lower,
                                     strlen (lower));
   json = ipc_data_to_json (hn);
-  ipc_data_destroy (hn);
+  ipc_data_destroy (&hn);
   if (plug_add_host_fqdn (lexic->script_infos, lower, source))
     goto end_add_hostname;
 

diff --git a/src/attack.c b/src/attack.c
@@ -501,7 +501,7 @@ read_ipc (struct ipc_context *ctx)
                 }
               break;
             }
-          ipc_data_destroy (idata);
+          ipc_data_destroy (&idata);
         }
     }
 }