fix: add defensive nullptr checks

Co-authored-by: Roberto Scolaro <[email protected]> Signed-off-by: Jason Dellaluce <[email protected]>
falcosecurity · Oct 20, 2023 · 0262474 · 0262474
1 parent 29d6baf
commit 0262474
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 3 deletions.
diff --git a/userspace/libsinsp/sinsp_filtercheck_evtin.cpp b/userspace/libsinsp/sinsp_filtercheck_evtin.cpp
@@ -248,7 +248,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p
 		//
 		// If this is a thread-related field, reject anything that doesn't come from the same thread
 		//
-		if(static_cast<int64_t>(pae->m_tid) != evt->get_thread_info()->m_tid)
+		auto* tinfo = evt->get_thread_info();
+		if(!tinfo || static_cast<int64_t>(pae->m_tid) != tinfo->m_tid)
 		{
 			return NULL;
 		}
@@ -262,7 +263,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p
 
 		if(tinfo)
 		{
-			if(tinfo->m_tid != evt->get_thread_info()->m_tid)
+			auto* evtinfo = evt->get_thread_info();
+			if(!evtinfo || tinfo->m_tid != evtinfo->m_tid)
 			{
 				return NULL;
 			}
@@ -283,7 +285,8 @@ inline uint8_t* sinsp_filter_check_evtin::extract_tracer(sinsp_evt *evt, sinsp_p
 
 		if(tinfo)
 		{
-			if(tinfo->m_pid != evt->get_thread_info()->m_ptid)
+			auto* evtinfo = evt->get_thread_info();
+			if(!evtinfo || tinfo->m_pid != evtinfo->m_ptid)
 			{
 				return NULL;
 			}

diff --git a/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp b/userspace/libsinsp/sinsp_filtercheck_fdlist.cpp
@@ -77,6 +77,10 @@ uint8_t* sinsp_filter_check_fdlist::extract(sinsp_evt *evt, OUT uint32_t* len, b
 	uint16_t nfds = *(uint16_t *)payload;
 	uint32_t pos = 2;
 	sinsp_threadinfo* tinfo = evt->get_thread_info();
+	if(!tinfo)
+	{
+		return NULL;
+	}
 
 	m_strval.clear();
 

diff --git a/userspace/libsinsp/test/classes/sinsp_thread_manager.cpp b/userspace/libsinsp/test/classes/sinsp_thread_manager.cpp
@@ -207,6 +207,7 @@ TEST_F(sinsp_with_test_input, THRD_MANAGER_find_reaper_in_the_tree)
 	DEFAULT_TREE
 
 	auto p6_t1_tinfo = m_inspector.get_thread_ref(p6_t1_tid, false).get();
+	ASSERT_TRUE(p6_t1_tinfo);
 
 	/* Call the find reaper method, the reaper for p6_t1 should be p4_t1  */
 	auto reaper = m_inspector.m_thread_manager->find_new_reaper(p6_t1_tinfo);

diff --git a/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp b/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp
@@ -89,6 +89,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_reaper)
 	DEFAULT_TREE
 
 	auto p3_t1_tinfo = m_inspector.get_thread_ref(p3_t1_tid, false).get();
+	ASSERT_NE(p3_t1_tinfo, nullptr);
 
 	/* The reaper cannot be the current process */
 	EXPECT_THROW(p3_t1_tinfo->assign_children_to_reaper(p3_t1_tinfo), sinsp_exception);
@@ -98,6 +99,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_reaper)
 	ASSERT_THREAD_CHILDREN(p1_t1_tid, 0, 0);
 
 	auto p1_t1_tinfo = m_inspector.get_thread_ref(p1_t1_tid, false).get();
+	ASSERT_NE(p1_t1_tinfo, nullptr);
 	p3_t1_tinfo->assign_children_to_reaper(p1_t1_tinfo);
 
 	/* all p3_t1 children should be removed */
@@ -120,6 +122,7 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_a_nullptr)
 	DEFAULT_TREE
 
 	auto p2_t1_tinfo = m_inspector.get_thread_ref(p2_t1_tid, false).get();
+	ASSERT_NE(p2_t1_tinfo, nullptr);
 	/* This call should change the parent of all children of p2_t1 to `0` */
 	p2_t1_tinfo->assign_children_to_reaper(nullptr);
 

diff --git a/userspace/libsinsp/test/events_proc.ut.cpp b/userspace/libsinsp/test/events_proc.ut.cpp
@@ -599,6 +599,7 @@ TEST_F(sinsp_with_test_input, last_exec_ts)
 	std::string argsv = test_utils::to_null_delimited(args);
 	evt = add_event_advance_ts(increasing_ts(), parent_tid, PPME_SYSCALL_CLONE_20_X, 20, child_tid, "bash", empty_bytebuf, parent_pid, parent_tid, 0, "", 1024, 0, 68633, 12088, 7208, 0, "bash", scap_const_sized_buffer{cgroupsv.data(), cgroupsv.size()}, PPM_CL_CLONE_CHILD_CLEARTID | PPM_CL_CLONE_CHILD_SETTID, 1000, 1000, parent_pid, parent_tid);
 
+	ASSERT_TRUE(evt->get_thread_info());
 	// Check we initialize lastexec time to zero
 	ASSERT_EQ(evt->get_thread_info()->m_lastexec_ts, 0);
 

diff --git a/userspace/libsinsp/test/scap_files/kexec_arm64/kexec_arm64.cpp b/userspace/libsinsp/test/scap_files/kexec_arm64/kexec_arm64.cpp
@@ -73,6 +73,7 @@ TEST(scap_file_kexec_arm64, tail_lineage)
 	std::vector<int64_t> expected_traverse_parents_after_execve = {tid_sh, tid_containerd_shim1, tid_systemd1,
 								       tid_containerd_shim2, tid_systemd2};
 	traverse_parents.clear();
+	ASSERT_TRUE(evt->get_thread_info());
 	evt->get_thread_info()->traverse_parent_state(visitor);
 	ASSERT_EQ(traverse_parents, expected_traverse_parents_after_execve);
 

diff --git a/userspace/libsinsp/test/scap_files/kexec_x86/kexec_x86.cpp b/userspace/libsinsp/test/scap_files/kexec_x86/kexec_x86.cpp
@@ -76,6 +76,7 @@ TEST(scap_file_kexec_x86, tail_lineage)
 	std::vector<int64_t> expected_traverse_parents_after_execve = {
 		tid_sh, tid_runc, tid_containerd_shim1, tid_systemd1, tid_containerd_shim2, tid_systemd2};
 	traverse_parents.clear();
+	ASSERT_TRUE(evt->get_thread_info());
 	evt->get_thread_info()->traverse_parent_state(visitor);
 	ASSERT_EQ(traverse_parents, expected_traverse_parents_after_execve);
 
@@ -99,6 +100,7 @@ TEST(scap_file_kexec_x86, tail_lineage)
 	std::vector<int64_t> expected_traverse_parents_after_remove = {tid_sh, tid_containerd_shim1, tid_systemd1,
 								       tid_containerd_shim2, tid_systemd2};
 	traverse_parents.clear();
+	ASSERT_TRUE(evt->get_thread_info());
 	evt->get_thread_info()->traverse_parent_state(visitor);
 	ASSERT_EQ(traverse_parents, expected_traverse_parents_after_remove);
 

diff --git a/userspace/libsinsp/test/thread_table.ut.cpp b/userspace/libsinsp/test/thread_table.ut.cpp
@@ -252,6 +252,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
 	/*=============================== p4_t1 traverse ===========================*/
 
 	sinsp_threadinfo* tinfo = m_inspector.get_thread_ref(p4_t1_tid, false, true).get();
+	ASSERT_TRUE(tinfo);
 
 	std::vector<int64_t> expected_p4_traverse_parents = {p4_t1_ptid, p3_t1_ptid, p2_t1_ptid};
 
@@ -264,6 +265,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
 	/*=============================== p5_t2 traverse ===========================*/
 
 	tinfo = m_inspector.get_thread_ref(p5_t2_tid, false).get();
+	ASSERT_TRUE(tinfo);
 
 	std::vector<int64_t> expected_p5_traverse_parents = {p5_t2_ptid, p4_t2_ptid, p3_t1_ptid, p2_t1_ptid};
 
@@ -321,6 +323,7 @@ TEST_F(sinsp_with_test_input, THRD_TABLE_traverse_default_tree)
 	/*=============================== p6_t1 traverse ===========================*/
 
 	tinfo = m_inspector.get_thread_ref(p6_t1_tid, false).get();
+	ASSERT_TRUE(tinfo);
 
 	std::vector<int64_t> expected_p6_traverse_parents = {p4_t1_tid, p2_t3_tid, INIT_TID};