diff --git a/libclamav_rust/src/css_image_extract.rs b/libclamav_rust/src/css_image_extract.rs index 8abd5dbf52..391d5bef23 100644 --- a/libclamav_rust/src/css_image_extract.rs +++ b/libclamav_rust/src/css_image_extract.rs @@ -152,7 +152,7 @@ impl<'a> CssImageExtractor<'a> { }; // Trim off " at end. - let c = url_parameter.graphemes(true).rev().next(); + let c = url_parameter.graphemes(true).next_back(); if let Some(c) = c { if c == "\"" { (url_parameter, _) = url_parameter.split_at(url_parameter.len() - 1); diff --git a/libclamav_rust/src/ctx.rs b/libclamav_rust/src/ctx.rs index 4dfff66e4e..e988238241 100644 --- a/libclamav_rust/src/ctx.rs +++ b/libclamav_rust/src/ctx.rs @@ -21,13 +21,14 @@ */ use std::convert::TryInto; +use std::path::{PathBuf}; use std::slice; use thiserror; use crate::fmap::FMap; use crate::sys::cli_ctx; -use crate::util::optional_cstring; +use crate::util::str_from_ptr; /// Error enumerates all possible errors returned by this library. #[derive(thiserror::Error, Debug)] @@ -46,6 +47,9 @@ pub enum Error { #[error("Invalid FMap")] BadMap, + + #[error("File path is not valid UTF8")] + FilepathNotUtf8, } /// Get the ctx.sub_filepath as an Option<'str> @@ -53,12 +57,14 @@ pub enum Error { /// # Safety /// /// Must be a valid ctx pointer. -pub fn sub_filepath(ctx: *mut cli_ctx) -> Result, Error> { +pub unsafe fn sub_filepath(ctx: *mut cli_ctx) -> Result, Error> { if ctx.is_null() { return Err(Error::NullPointer("ctx")); } - Ok(optional_cstring(unsafe { *ctx }.sub_filepath)) + Ok(str_from_ptr(unsafe { *ctx }.sub_filepath) + .map_err(|_| Error::FilepathNotUtf8)? + .map(PathBuf::from)) } /// Get the ctx.target_filepath as an Option<'str> @@ -66,12 +72,14 @@ pub fn sub_filepath(ctx: *mut cli_ctx) -> Result, Error> { /// # Safety /// /// Must be a valid ctx pointer. -pub fn target_filepath(ctx: *mut cli_ctx) -> Result, Error> { +pub unsafe fn target_filepath(ctx: *mut cli_ctx) -> Result, Error> { if ctx.is_null() { return Err(Error::NullPointer("ctx")); } - Ok(optional_cstring(unsafe { *ctx }.target_filepath)) + Ok(str_from_ptr(unsafe { *ctx }.target_filepath) + .map_err(|_| Error::FilepathNotUtf8)? + .map(PathBuf::from)) } /// Get the fmap for the current layer. @@ -79,7 +87,7 @@ pub fn target_filepath(ctx: *mut cli_ctx) -> Result, Error> /// # Safety /// /// Must be a valid ctx pointer. -pub fn current_fmap(ctx: *mut cli_ctx) -> Result { +pub unsafe fn current_fmap(ctx: *mut cli_ctx) -> Result { if ctx.is_null() { return Err(Error::NullPointer("ctx")); } diff --git a/libclamav_rust/src/fmap.rs b/libclamav_rust/src/fmap.rs index f1b9b153f5..380d1a7a39 100644 --- a/libclamav_rust/src/fmap.rs +++ b/libclamav_rust/src/fmap.rs @@ -25,7 +25,7 @@ use std::convert::TryFrom; use log::{debug, error}; use thiserror::Error; -use crate::{sys, util::optional_cstring}; +use crate::{sys, util::str_from_ptr}; /// FMapError enumerates all possible errors returned by this library. #[derive(Error, Debug)] @@ -71,7 +71,7 @@ impl<'a> FMap { let ptr: *const u8 = unsafe { need_fn(self.fmap_ptr, at, len, 1) } as *const u8; - if ptr == std::ptr::null() { + if ptr.is_null() { let fmap_size = unsafe { *self.fmap_ptr }.len; debug!( "need_off at {:?} len {:?} for fmap size {:?} returned NULL", @@ -96,7 +96,15 @@ impl<'a> FMap { unsafe { (*self.fmap_ptr).len } } + pub fn is_empty(&self) -> bool { + unsafe { (*self.fmap_ptr).len == 0 } + } + pub fn name(&self) -> &'static str { - optional_cstring(unsafe { (*self.fmap_ptr).name }).unwrap_or("") + let name_result = unsafe { str_from_ptr((*self.fmap_ptr).name) }; + match name_result { + Ok(Some(name)) => name, + _ => "", + } } } diff --git a/libclamav_rust/src/onenote.rs b/libclamav_rust/src/onenote.rs index 1b69105340..e1187760b5 100644 --- a/libclamav_rust/src/onenote.rs +++ b/libclamav_rust/src/onenote.rs @@ -55,8 +55,8 @@ pub enum Error { #[error("Unable to parse OneNote file")] Parse, - #[error("Failed to load image due to bug in image decoder")] - SectionParsePanic(), + #[error("Failed to parse OneNote file due to a panic in the onenote_parser library")] + OneNoteParserPanic, } fn find_bytes(haystack: &[u8], needle: &[u8]) -> Option { @@ -137,7 +137,7 @@ impl<'a> OneNote<'a> { }; embedded_files.push(ExtractedFile { - name: name, + name, data: data.to_vec(), }); } @@ -158,11 +158,11 @@ impl<'a> OneNote<'a> { // Try to parse the section buffer using the onenote_parser crate. // Attempt to catch panics in case the parser encounter unexpected issues. let result_result = panic::catch_unwind(|| -> Result, Error> { - Ok(parse_section_buffer(data, filename)?) + parse_section_buffer(data, filename) }); // Check if it panicked. If no panic, grab the parse result. - let result = result_result.map_err(|_| Error::Parse)?; + let result = result_result.map_err(|_| Error::OneNoteParserPanic)?; if let Ok(embedded_files) = result { // Successfully parsed the OneNote file with the onenote_parser crate. @@ -188,6 +188,7 @@ impl<'a> OneNote<'a> { } } + /// Open a OneNote document given the document was provided as a slice of bytes. pub fn next_file(&mut self) -> Option { debug!("Looking to extract file from OneNote section..."); @@ -208,24 +209,22 @@ impl<'a> OneNote<'a> { | ((data_length_bytes[1] as u64) << 8) | (data_length_bytes[0] as u64); - let data: &[u8] = &remaining[SIZE_OF_FILE_DATA_HEADER - ..SIZE_OF_FILE_DATA_HEADER + data_length as usize]; + let data: &[u8] = &remaining + [SIZE_OF_FILE_DATA_HEADER..SIZE_OF_FILE_DATA_HEADER + data_length as usize]; file_data = Some(data.to_vec()); - Some( - &remaining - [SIZE_OF_FILE_DATA_HEADER + (data_length as usize)..remaining.len()], - ) + Some(&remaining[SIZE_OF_FILE_DATA_HEADER + (data_length as usize)..remaining.len()]) } else { None }; self.remaining = remaining; - file_data.map(|data| ExtractedFile { data, name: None, }) + file_data.map(|data| ExtractedFile { data, name: None }) } + /// Get the next file from the OneNote document using the method required for when we've read the file into a Vec. pub fn next_file_vec(&mut self) -> Option { debug!("Looking to extract file from OneNote section..."); @@ -246,22 +245,22 @@ impl<'a> OneNote<'a> { | ((data_length_bytes[1] as u64) << 8) | (data_length_bytes[0] as u64); - let data: &[u8] = &remaining[SIZE_OF_FILE_DATA_HEADER - ..SIZE_OF_FILE_DATA_HEADER + data_length as usize]; + let data: &[u8] = &remaining + [SIZE_OF_FILE_DATA_HEADER..SIZE_OF_FILE_DATA_HEADER + data_length as usize]; file_data = Some(data.to_vec()); Some(Vec::from( - &remaining - [SIZE_OF_FILE_DATA_HEADER + (data_length as usize)..remaining.len()], + &remaining[SIZE_OF_FILE_DATA_HEADER + (data_length as usize)..remaining.len()], )) } else { None }; - file_data.map(|data| ExtractedFile { data, name: None, }) + file_data.map(|data| ExtractedFile { data, name: None }) } + /// Get the next file from the OneNote document using the method required for the onenote_parser crate. pub fn next_file_parser(&mut self) -> Option { self.embedded_files.pop() } @@ -278,7 +277,7 @@ impl<'a> Iterator for OneNote<'a> { } else if self.remaining_vec.is_some() { // Data stored in a Vec. self.next_file_vec() - } else if self.embedded_files.len() > 0 { + } else if !self.embedded_files.is_empty() { // Data stored in a Vec. self.next_file_parser() } else { diff --git a/libclamav_rust/src/scanners.rs b/libclamav_rust/src/scanners.rs index ab88ff2a2e..07dc42444e 100644 --- a/libclamav_rust/src/scanners.rs +++ b/libclamav_rust/src/scanners.rs @@ -66,18 +66,22 @@ fn magic_scan(ctx: *mut cli_ctx, buf: &[u8], name: Option) -> cl_error_t } // Okay now safe to drop the name CString. - drop(unsafe { CString::from_raw(name_ptr) }); + let _ = unsafe { CString::from_raw(name_ptr) }; ret } /// Scan a OneNote file for attachments +/// +/// # Safety +/// +/// Must be a valid ctx pointer. #[no_mangle] pub unsafe extern "C" fn scan_onenote(ctx: *mut cli_ctx) -> cl_error_t { let fmap = match ctx::current_fmap(ctx) { Ok(fmap) => fmap, Err(e) => { - warn!("Error getting FMap from ctx: {}", e); + warn!("Error getting FMap from ctx: {e}"); return cl_error_t_CL_ERROR; } }; @@ -86,17 +90,14 @@ pub unsafe extern "C" fn scan_onenote(ctx: *mut cli_ctx) -> cl_error_t { Ok(bytes) => bytes, Err(err) => { error!( - "Failed to get file bytes for fmap of size {}: {}", - fmap.len(), - err.to_string() + "Failed to get file bytes for fmap of size {}: {err}", + fmap.len() ); return cl_error_t_CL_ERROR; } }; - let one = OneNote::from_bytes(file_bytes, &Path::new(fmap.name())); - - let one = match one { + let one = match OneNote::from_bytes(file_bytes, Path::new(fmap.name())) { Ok(x) => x, Err(err) => { error!("Failed to parse OneNote file: {}", err.to_string()); @@ -104,7 +105,7 @@ pub unsafe extern "C" fn scan_onenote(ctx: *mut cli_ctx) -> cl_error_t { } }; - one.into_iter().enumerate().all(|(_, attachment)| { + one.into_iter().all(|attachment| { debug!( "Extracted {}-byte attachment with name: {:?}", attachment.data.len(), @@ -112,7 +113,6 @@ pub unsafe extern "C" fn scan_onenote(ctx: *mut cli_ctx) -> cl_error_t { ); let ret = magic_scan(ctx, &attachment.data, attachment.name); - if ret != cl_error_t_CL_SUCCESS { warn!("cl_scandesc_magic returned error: {}", ret); return false; diff --git a/libclamav_rust/src/util.rs b/libclamav_rust/src/util.rs index 6d182ca92d..9b8fb0c457 100644 --- a/libclamav_rust/src/util.rs +++ b/libclamav_rust/src/util.rs @@ -20,7 +20,6 @@ * MA 02110-1301, USA. */ -use log::warn; use std::ffi::CStr; use std::fs::File; @@ -55,16 +54,12 @@ pub fn file_from_fd_or_handle(fd: i32) -> File { /// /// The caller is responsible for making sure the lifetime of the pointer /// exceeds the lifetime of the output string. -pub fn optional_cstring(ptr: *const i8) -> Option<&'static str> { +/// +/// ptr must be a valid pointer to a C string. +pub unsafe fn str_from_ptr(ptr: *const i8) -> Result, std::str::Utf8Error> { if ptr.is_null() { - return None; + return Ok(None); } - match unsafe { CStr::from_ptr(ptr) }.to_str() { - Err(e) => { - warn!("{} is not valid unicode: {}", stringify!(ptr), e); - return None; - } - Ok(s) => Some(s), - } + Some(unsafe { CStr::from_ptr(ptr) }.to_str()).transpose() }