From 63d5257ca6f5da02749419d08384f7755b9d6c76 Mon Sep 17 00:00:00 2001 From: Benjamin Leggett Date: Wed, 22 Jul 2026 17:25:45 -0400 Subject: [PATCH] fix(plugin): get rid of several `expects` --- src/base_plugin/mod.rs | 63 +++++++++++++++++++++++++++--------------- src/parsers.rs | 19 ++++++++++--- src/threadstate.rs | 9 +++--- 3 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/base_plugin/mod.rs b/src/base_plugin/mod.rs index 315bfaf..6b8385e 100644 --- a/src/base_plugin/mod.rs +++ b/src/base_plugin/mod.rs @@ -32,6 +32,16 @@ const STDIN_FD: u64 = 0; const STDOUT_FD: u64 = 1; const STDERR_FD: u64 = 2; +/// Build a CString from a zone-provided String, truncating at the first interior +/// NUL (C-string semantics), so conversion always truncates instead of panicking. +fn cstring_lossy(s: impl Into>) -> CString { + let mut bytes = s.into(); + bytes.push(0); // guarantee a terminator so from_bytes_until_nul always succeeds + CStr::from_bytes_until_nul(&bytes) + .expect("terminator was just appended") + .to_owned() +} + // TODO(bml) we do not need a thread table entry now. // but we will for the container plugin. Defer until then. @@ -126,13 +136,13 @@ pub struct EderaZoneSyscallContext { impl EderaPlugin { pub fn extract_zone_id(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - Ok(CString::new(zone_evt.zone_id.clone()).expect("should cstring")) + Ok(cstring_lossy(zone_evt.zone_id.clone())) }) } pub fn extract_type(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - Ok(CString::new(zone_evt.event_name.clone()).expect("should cstring")) + Ok(cstring_lossy(zone_evt.event_name.clone())) }) } @@ -144,7 +154,7 @@ impl EderaPlugin { pub fn extract_category(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - Ok(CString::new(zone_evt.event_category.clone()).expect("should cstring")) + Ok(cstring_lossy(zone_evt.event_category.clone())) }) } @@ -190,14 +200,14 @@ impl EderaPlugin { ); concat_str.push_str("| "); } - Ok(CString::new(concat_str).expect("should cstring")) + Ok(cstring_lossy(concat_str)) }) } pub fn extract_arg(&mut self, mut req: ExtractRequest, arg: u64) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { if let Some(param) = zone_evt.event_params.get(arg as usize) { - Ok(CString::new(param.param_pretty.clone()).expect("should cstring")) + Ok(cstring_lossy(param.param_pretty.clone())) } else { Ok(CString::new("").unwrap()) } @@ -349,8 +359,7 @@ impl EderaPlugin { pub fn extract_count_error_file(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { let etype = event_codes::from_repr(zone_evt.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + .ok_or_else(|| anyhow!("could not parse event type"))?; if parsers::is_open_file(etype) || etype == event_codes::PPME_SYSCALL_CREAT_X { Self::get_count_error(zone_evt) } else { @@ -365,8 +374,7 @@ impl EderaPlugin { pub fn extract_count_error_net(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { let etype = event_codes::from_repr(zone_evt.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + .ok_or_else(|| anyhow!("could not parse event type"))?; if parsers::is_open_net(etype) { Self::get_count_error(zone_evt) } else { @@ -389,8 +397,7 @@ impl EderaPlugin { pub fn extract_count_error_other(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { let etype = event_codes::from_repr(zone_evt.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + .ok_or_else(|| anyhow!("could not parse event type"))?; if !parsers::is_open_file(etype) && !parsers::is_open_net(etype) && !zone_evt.event_category.contains("MEMORY") @@ -2661,9 +2668,9 @@ impl EderaPlugin { } fn l4proto_to_string(l4proto: u32) -> String { - let proto = l4_types::from_repr(l4proto) - .ok_or(anyhow!("could not parse proto type")) - .expect("should parse"); + let Some(proto) = l4_types::from_repr(l4proto) else { + return "NA".to_string(); + }; match proto { l4_types::SCAP_L4_TCP => "tcp".into(), l4_types::SCAP_L4_UDP => "udp".into(), @@ -2711,9 +2718,9 @@ impl EderaPlugin { pub fn extract_raw_fdname_from_event(event: &ZoneKernelSyscallEvent) -> String { use event_codes::*; - let etype = event_codes::from_repr(event.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + let Some(etype) = event_codes::from_repr(event.event_type) else { + return "NA".to_string(); + }; if parsers::is_enter(event) { return "NA".into(); @@ -2791,9 +2798,9 @@ impl EderaPlugin { fn get_paths_from_evt_params(event: &ZoneKernelSyscallEvent) -> Vec { let mut paths = Vec::new(); use event_codes::*; - let etype = event_codes::from_repr(event.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + let Some(etype) = event_codes::from_repr(event.event_type) else { + return paths; + }; match etype { // Single path operations PPME_SYSCALL_MKDIR_2_X => { @@ -2946,9 +2953,9 @@ impl EderaPlugin { fn get_fdlist_from_poll_evt(event: &ZoneKernelSyscallEvent) -> Vec { let mut poll_fds = Vec::new(); use event_codes::*; - let etype = event_codes::from_repr(event.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + let Some(etype) = event_codes::from_repr(event.event_type) else { + return poll_fds; + }; let fddata = match etype { PPME_SYSCALL_PPOLL_E => event.event_params[0].param_data.clone(), PPME_SYSCALL_PPOLL_X => event.event_params[1].param_data.clone(), @@ -3091,4 +3098,14 @@ mod tests { .collect(); assert_eq!(paths, vec!["/opt/host-canary".to_string()]); } + + #[test] + fn cstring_lossy_truncates_at_interior_nul() { + // An interior NUL in a wire string must not panic. + assert_eq!( + cstring_lossy("execve\0evil".to_string()).to_bytes(), + b"execve" + ); + assert_eq!(cstring_lossy("clean".to_string()).to_bytes(), b"clean"); + } } diff --git a/src/parsers.rs b/src/parsers.rs index 00e7ed2..c2c690c 100644 --- a/src/parsers.rs +++ b/src/parsers.rs @@ -2,7 +2,7 @@ use crate::proto::generated::protect::control::v1::ZoneKernelFdInfo; use crate::proto::generated::protect::control::v1::{ ZoneKernelSyscallEvent, ZoneKernelThreadInfo, zone_kernel_fd_info_data::InfoType, }; -use anyhow::{Result, anyhow}; +use anyhow::Result; use libscap_bindings::consts as ppm_consts; use libscap_bindings::types::{ ppm_event_code as event_codes, ppm_event_flags as event_flags, ppm_param_type as param_type, @@ -228,9 +228,7 @@ pub fn get_fdi(event: &ZoneKernelSyscallEvent) -> Option { return None; } - let etype = event_codes::from_repr(event.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + let etype = event_codes::from_repr(event.event_type)?; // For exit events (modern_bpf only captures these), search for the FD parameter // Special case: sendmmsg and recvmmsg have FD at position 1 @@ -531,4 +529,17 @@ mod tests { assert!(!is_open_create(&evt)); assert!(!is_open_exec(&evt)); } + + #[test] + fn get_fdi_unknown_event_type_does_not_panic() { + // A malicious zone can send an arbitrary event_type, so classification must + // return None rather than panic. + let evt = ZoneKernelSyscallEvent { + event_type: 0xDEAD_BEEF, + event_flags: event_flags::EF_USES_FD as u32, + ..Default::default() + }; + assert!(has_fd(&evt)); + assert_eq!(get_fdi(&evt), None); + } } diff --git a/src/threadstate.rs b/src/threadstate.rs index 1c685cc..4563048 100644 --- a/src/threadstate.rs +++ b/src/threadstate.rs @@ -1,4 +1,4 @@ -use anyhow::{Result, anyhow}; +use anyhow::Result; use libc::{ AF_INET, AF_INET6, IPPROTO_ICMP, IPPROTO_IP, IPPROTO_RAW, IPPROTO_TCP, IPPROTO_UDP, SOCK_DGRAM, SOCK_RAW, SOCK_STREAM, @@ -3190,9 +3190,10 @@ impl ThreadState { /// See `sinsp_parser::process_event` in libsinsp/parsers.cpp pub fn process_event(&mut self, event: &ZoneKernelSyscallEvent) -> Result<()> { use event_codes::*; - let etype = event_codes::from_repr(event.event_type) - .ok_or(anyhow!("could not parse event type")) - .expect("should parse"); + let Some(etype) = event_codes::from_repr(event.event_type) else { + debug!("ignoring event with unknown type {}", event.event_type); + return Ok(()); + }; let Some(zinfo) = self.zone_info.get_mut(&event.zone_id) else { debug!("ignoring event for unmonitored zone {:?}", &event.zone_id);