cleanup(libsinsp): throw exception for invalid parsed string vectors

Signed-off-by: Luca Guerra <[email protected]>

Author: LucaGuerra

userspace/libsinsp/event.h

@@ -32,6 +32,7 @@ limitations under the License.
 #include <libsinsp/settings.h>
 #include <libsinsp/sinsp_exception.h>
 #include <libsinsp/fdinfo.h>
+#include <libsinsp/utils.h>
 
 class sinsp;
 class sinsp_threadinfo;
@@ -150,6 +151,12 @@ inline std::string_view get_event_param_as<std::string_view>(const sinsp_evt_par
 	return {param.m_val, string_len};
 }
 
+template<>
+inline std::vector<std::string> get_event_param_as<std::vector<std::string>>(const sinsp_evt_param& param)
+{
+	return sinsp_split(param.m_val, static_cast<size_t>(param.m_len), '\0');
+}
+
 /*!
   \brief Event class.
   This class is returned by \ref sinsp::next() and encapsulates the state

userspace/libsinsp/parsers.cpp

@@ -940,7 +940,6 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev
 
 void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
 {
-	const sinsp_evt_param* parinfo = nullptr;
 	uint16_t etype = evt->get_type();
 	int64_t caller_tid = evt->get_tid();
 
@@ -1229,8 +1228,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
 	child_tinfo->m_exe = evt->get_param(1)->as<std::string_view>();
 
 	/* args */
-	parinfo = evt->get_param(2);
-	child_tinfo->set_args(parinfo->m_val, parinfo->m_len);
+	child_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
 
 	/* comm */
 	switch(etype)
@@ -1354,8 +1352,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
 		case PPME_SYSCALL_VFORK_20_X:
 		case PPME_SYSCALL_CLONE_20_X:
 		case PPME_SYSCALL_CLONE3_X:
-			parinfo = evt->get_param(14);
-			child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len);
+			child_tinfo->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());
 			m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live() || m_inspector->is_syscall_plugin());
 			break;
 	}
@@ -1411,8 +1408,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
 		caller_tinfo->m_comm = child_tinfo->m_comm;
 
 		/* args */
-		parinfo = evt->get_param(2);
-		caller_tinfo->set_args(parinfo->m_val, parinfo->m_len);
+		caller_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
 	}
 
 	/*=============================== CREATE CHILD ===========================*/
@@ -1459,7 +1455,6 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid)
 
 void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
 {
-	const sinsp_evt_param *parinfo = nullptr;
 	uint16_t etype = evt->get_type();
 	int64_t child_tid = evt->get_tid();
 
@@ -1688,8 +1683,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
 	}
 
 	/* args */
-	parinfo = evt->get_param(2);
-	child_tinfo->set_args(parinfo->m_val, parinfo->m_len);
+	child_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
 
 	if(valid_lookup_thread)
 	{
@@ -1799,8 +1793,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
 			lookup_tinfo->m_comm = child_tinfo->m_comm;
 
 			/* args */
-			parinfo = evt->get_param(2);
-			lookup_tinfo->set_args(parinfo->m_val, parinfo->m_len);
+			lookup_tinfo->set_args(evt->get_param(2)->as<std::vector<std::string>>());
 		}
 	}
 
@@ -1904,8 +1897,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt)
 	case PPME_SYSCALL_VFORK_20_X:
 	case PPME_SYSCALL_CLONE_20_X:
 	case PPME_SYSCALL_CLONE3_X:
-		parinfo = evt->get_param(14);
-		child_tinfo->set_cgroups(parinfo->m_val, parinfo->m_len);
+		child_tinfo->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());
 		m_inspector->m_container_manager.resolve_container(child_tinfo.get(), m_inspector->is_live());
 		break;
 	}
@@ -2080,8 +2072,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
 	}
 
 	// Get the command arguments
-	parinfo = evt->get_param(2);
-	evt->get_tinfo()->set_args(parinfo->m_val, parinfo->m_len);
+	evt->get_tinfo()->set_args(evt->get_param(2)->as<std::vector<std::string>>());
 
 	// Get the pid
 	evt->get_tinfo()->m_pid = evt->get_param(4)->as<uint64_t>();
@@ -2168,8 +2159,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
 		//
 		// Set cgroups and heuristically detect container id
 		//
-		parinfo = evt->get_param(14);
-		evt->get_tinfo()->set_cgroups(parinfo->m_val, parinfo->m_len);
+		evt->get_tinfo()->set_cgroups(evt->get_param(14)->as<std::vector<std::string>>());
 
 		//
 		// Resync container status after an execve, we need to do it

userspace/libsinsp/test/sinsp_utils.ut.cpp

@@ -197,3 +197,14 @@ TEST(sinsp_utils_test, concatenate_paths)
 	res = sinsp_utils::concatenate_paths(path1, path2);
 	EXPECT_EQ("/root/c:/hello/world", res); */
 }
+
+TEST(sinsp_utils_test, sinsp_split_buf)
+{
+	const char *in = "hello\0world\0";
+	size_t len = 12;
+	auto split = sinsp_split(in, len, '\0');
+
+	EXPECT_EQ(split.size(), 2);
+	EXPECT_EQ(split[0], "hello");
+	EXPECT_EQ(split[1], "world");
+}

userspace/libsinsp/threadinfo.cpp

@@ -616,14 +616,12 @@ std::string sinsp_threadinfo::get_exepath() const
 
 void sinsp_threadinfo::set_args(const char* args, size_t len)
 {
-	m_args.clear();
+	set_args(sinsp_split(args, len, '\0'));
+}
 
-	size_t offset = 0;
-	while(offset < len)
-	{
-		m_args.push_back(args + offset);
-		offset += m_args.back().length() + 1;
-	}
+void sinsp_threadinfo::set_args(std::vector<std::string> args)
+{
+	m_args = args;
 }
 
 void sinsp_threadinfo::set_env(const char* env, size_t len)
@@ -641,32 +639,7 @@ void sinsp_threadinfo::set_env(const char* env, size_t len)
 		}
 	}
 
-	m_env.clear();
-	size_t offset = 0;
-	while(offset < len)
-	{
-		const char* left = env + offset;
-		// environment string may actually be shorter than indicated by len
-		// if the rest is empty, we bail out early
-		if(!strlen(left))
-		{
-			size_t sz = len - offset;
-			void* zero = calloc(sz, sizeof(char));
-			if(zero == NULL)
-			{
-				throw sinsp_exception("memory allocation error in sinsp_threadinfo::set_env");
-			}
-			if(!memcmp(left, zero, sz))
-			{
-				free(zero);
-				return;
-			}
-			free(zero);
-		}
-		m_env.push_back(left);
-
-		offset += m_env.back().length() + 1;
-	}
+	m_env = sinsp_split(env, len, '\0');
 }
 
 bool sinsp_threadinfo::set_env_from_proc() {
@@ -756,24 +729,25 @@ std::string sinsp_threadinfo::concatenate_all_env()
 }
 
 void sinsp_threadinfo::set_cgroups(const char* cgroups, size_t len)
+{
+	set_cgroups(sinsp_split(cgroups, len, '\0'));
+}
+
+void sinsp_threadinfo::set_cgroups(std::vector<std::string> cgroups)
 {
 	decltype(m_cgroups) tmp_cgroups(new cgroups_t);
 
-	size_t offset = 0;
-	while(offset < len)
+	for( auto &def : cgroups)
 	{
-		const char* str = cgroups + offset;
-		const char* sep = strrchr(str, '=');
-		if(sep == NULL)
+		std::string::size_type eq_pos = def.find("=");
+		if (eq_pos == std::string::npos)
 		{
-			ASSERT(false);
 			return;
 		}
 
-		std::string subsys(str, sep - str);
-		std::string cgroup(sep + 1);
+		std::string subsys = def.substr(0, eq_pos);
+		std::string cgroup = def.substr(eq_pos + 1);
 
-		size_t subsys_length = subsys.length();
 		size_t pos = subsys.find("_cgroup");
 		if(pos != std::string::npos)
 		{
@@ -797,7 +771,6 @@ void sinsp_threadinfo::set_cgroups(const char* cgroups, size_t len)
 		}
 
 		tmp_cgroups->push_back(std::make_pair(subsys, cgroup));
-		offset += subsys_length + 1 + cgroup.length() + 1;
 	}
 
 	m_cgroups.swap(tmp_cgroups);

userspace/libsinsp/threadinfo.h

@@ -638,8 +638,10 @@ class SINSP_PUBLIC sinsp_threadinfo : public libsinsp::state::table_entry
 	void remove_fd(int64_t fd);
 	void update_cwd(std::string_view cwd);
 	void set_args(const char* args, size_t len);
+	void set_args(std::vector<std::string> args);
 	void set_env(const char* env, size_t len);
 	void set_cgroups(const char* cgroups, size_t len);
+	void set_cgroups(std::vector<std::string> cgroups);
 	bool is_lastevent_data_valid() const;
 	inline void set_lastevent_data_validity(bool isvalid)
 	{

userspace/libsinsp/utils.cpp

@@ -1385,6 +1385,24 @@ std::vector<std::string> sinsp_split(const std::string &s, char delim)
 	return res;
 }
 
+std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim)
+{
+	if(len == 0)
+	{
+		return {};
+	}
+
+	if(buf[len - 1] != '\0')
+	{
+		throw sinsp_exception("expected a NUL-terminated buffer of size " +	
+							  std::to_string(len) + ", which instead ends with " +
+							  std::to_string(buf[len - 1]));
+	}
+
+	std::string s {buf, len - 1};
+	return sinsp_split(s, delim);
+}
+
 //
 // trim from start
 //

userspace/libsinsp/utils.h

@@ -26,6 +26,7 @@ limitations under the License.
 
 #include <algorithm>
 #include <cctype>
+#include <cstddef>
 #include <cstring>
 #include <list>
 #include <locale>
@@ -257,6 +258,7 @@ const char* print_format_to_string(ppm_print_format fmt);
 // String helpers
 ///////////////////////////////////////////////////////////////////////////////
 std::vector<std::string> sinsp_split(const std::string& s, char delim);
+std::vector<std::string> sinsp_split(const char *buf, size_t len, char delim);
 
 template<typename It>
 std::string sinsp_join(It begin, It end, char delim)