From 5eeec5188177b03a3c3fd44cb2f32cf43fc54949 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:13:21 -0400 Subject: [PATCH 1/6] adding safety checks to support calls to from_raw_parts() --- src/parser.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index c7b7683..f718972 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -372,16 +372,33 @@ macro_rules! impl_try_parse_primitive_array { PropertyInfo::Array { .. } => { // TODO: Check In and Out type and do a better type checking let size = std::mem::size_of::<$T>(); + let align = std::mem::align_of::<$T>(); + if prop_slice.buffer.len() % size != 0 { return Err(ParserError::LengthMismatch); } + let count = prop_slice.buffer.len() / size; + + if prop_slice.buffer.as_ptr() as usize % align != 0 { + return Err(ParserError::PropertyError( + "buffer alignment mismatch".into() + )); + } + + if size.checked_mul(count).is_none() || (size * count) > isize::MAX as usize { + return Err(ParserError::PropertyError( + "size overflow".into() + )); + } + let slice = unsafe { std::slice::from_raw_parts( prop_slice.buffer.as_ptr() as *const $T, count, ) }; + Ok(slice) } _ => Err(ParserError::InvalidType), @@ -441,12 +458,20 @@ impl private::TryParse for Parser<'_, '_> { match prop_slice.property.info { PropertyInfo::Value { in_type, .. } => match in_type { TdhInType::InTypeUnicodeString => { + let align = std::mem::align_of::(); + if prop_slice.buffer.len() % 2 != 0 { return Err(ParserError::PropertyError( "odd length in bytes for a wide string".into(), )); } + if prop_slice.buffer.as_ptr() as usize % align != 0 { + return Err(ParserError::PropertyError( + "buffer alignment mismatch".into(), + )); + } + let mut wide = unsafe { std::slice::from_raw_parts( prop_slice.buffer.as_ptr() as *const u16, @@ -460,6 +485,7 @@ impl private::TryParse for Parser<'_, '_> { _ => (), } + // Decode UTF-16 to String Ok(widestring::decode_utf16_lossy(wide.iter().copied()).collect::()) } TdhInType::InTypeAnsiString => { From 1877643e952645e66a773d8058634bf265131f12 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:25:48 -0400 Subject: [PATCH 2/6] adding temp debug prints --- src/parser.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/parser.rs b/src/parser.rs index f718972..1cee4a5 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -375,18 +375,21 @@ macro_rules! impl_try_parse_primitive_array { let align = std::mem::align_of::<$T>(); if prop_slice.buffer.len() % size != 0 { + println!("[ferrisetw] length mismatch"); return Err(ParserError::LengthMismatch); } let count = prop_slice.buffer.len() / size; if prop_slice.buffer.as_ptr() as usize % align != 0 { + println!("[ferrisetw] buffer alignment mismatch"); return Err(ParserError::PropertyError( "buffer alignment mismatch".into() )); } if size.checked_mul(count).is_none() || (size * count) > isize::MAX as usize { + println!("[ferrisetw] size overflow"); return Err(ParserError::PropertyError( "size overflow".into() )); @@ -461,12 +464,14 @@ impl private::TryParse for Parser<'_, '_> { let align = std::mem::align_of::(); if prop_slice.buffer.len() % 2 != 0 { + println!("[ferrisetw] odd length in bytes for a wide string"); return Err(ParserError::PropertyError( "odd length in bytes for a wide string".into(), )); } if prop_slice.buffer.as_ptr() as usize % align != 0 { + println!("[ferrisetw] buffer alignment mismatch"); return Err(ParserError::PropertyError( "buffer alignment mismatch".into(), )); From 3ba50fe76c4e937d7a843a4940d3e86ba37d4780 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:33:54 -0400 Subject: [PATCH 3/6] more triaging --- src/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser.rs b/src/parser.rs index 1cee4a5..8958c0d 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -471,7 +471,7 @@ impl private::TryParse for Parser<'_, '_> { } if prop_slice.buffer.as_ptr() as usize % align != 0 { - println!("[ferrisetw] buffer alignment mismatch"); + println!("[ferrisetw] buffer alignment mismatch. align: {} size: {}", align, prop_slice.buffer.as_ptr() as usize); return Err(ParserError::PropertyError( "buffer alignment mismatch".into(), )); From e168405e055b51b0ae68940db67a100f28b3f994 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:38:14 -0400 Subject: [PATCH 4/6] temporarily removing alignment check for strings --- src/parser.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 8958c0d..ee1b8d4 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -461,7 +461,7 @@ impl private::TryParse for Parser<'_, '_> { match prop_slice.property.info { PropertyInfo::Value { in_type, .. } => match in_type { TdhInType::InTypeUnicodeString => { - let align = std::mem::align_of::(); + //let align = std::mem::align_of::(); if prop_slice.buffer.len() % 2 != 0 { println!("[ferrisetw] odd length in bytes for a wide string"); @@ -470,12 +470,12 @@ impl private::TryParse for Parser<'_, '_> { )); } - if prop_slice.buffer.as_ptr() as usize % align != 0 { - println!("[ferrisetw] buffer alignment mismatch. align: {} size: {}", align, prop_slice.buffer.as_ptr() as usize); - return Err(ParserError::PropertyError( - "buffer alignment mismatch".into(), - )); - } + // if prop_slice.buffer.as_ptr() as usize % align != 0 { + // println!("[ferrisetw] buffer alignment mismatch. align: {} size: {}", align, prop_slice.buffer.as_ptr() as usize); + // return Err(ParserError::PropertyError( + // "buffer alignment mismatch".into(), + // )); + // } let mut wide = unsafe { std::slice::from_raw_parts( From 61a58cb2514f17af1254ea407af7a9ff9169ce90 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:45:50 -0400 Subject: [PATCH 5/6] trying to use Vec to fix pointer alignment weirdness --- src/parser.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index ee1b8d4..1034721 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -470,6 +470,14 @@ impl private::TryParse for Parser<'_, '_> { )); } + let mut aligned_buffer = Vec::with_capacity(prop_slice.buffer.len() / 2); + for chunk in prop_slice.buffer.chunks_exact(2) { + let part = u16::from_ne_bytes([chunk[0], chunk[1]]); + aligned_buffer.push(part); + } + + + // if prop_slice.buffer.as_ptr() as usize % align != 0 { // println!("[ferrisetw] buffer alignment mismatch. align: {} size: {}", align, prop_slice.buffer.as_ptr() as usize); // return Err(ParserError::PropertyError( @@ -477,12 +485,14 @@ impl private::TryParse for Parser<'_, '_> { // )); // } - let mut wide = unsafe { - std::slice::from_raw_parts( - prop_slice.buffer.as_ptr() as *const u16, - prop_slice.buffer.len() / 2, - ) - }; + // let mut wide = unsafe { + // std::slice::from_raw_parts( + // prop_slice.buffer.as_ptr() as *const u16, + // prop_slice.buffer.len() / 2, + // ) + // }; + + let mut wide = aligned_buffer.as_slice(); match wide.last() { // remove the null terminator from the slice From 3859982c66e097478a05a90e2adbeb62de1b0647 Mon Sep 17 00:00:00 2001 From: matterpreter Date: Thu, 2 May 2024 15:56:58 -0400 Subject: [PATCH 6/6] cleanup --- src/parser.rs | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index 1034721..75b2a17 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -371,25 +371,26 @@ macro_rules! impl_try_parse_primitive_array { match prop_slice.property.info { PropertyInfo::Array { .. } => { // TODO: Check In and Out type and do a better type checking + + // This property type has not been tested yet as I don't have a + // provider that uses it. It's possible that the buffer is not + // aligned correctly, which would cause this to fail. let size = std::mem::size_of::<$T>(); let align = std::mem::align_of::<$T>(); if prop_slice.buffer.len() % size != 0 { - println!("[ferrisetw] length mismatch"); return Err(ParserError::LengthMismatch); } let count = prop_slice.buffer.len() / size; if prop_slice.buffer.as_ptr() as usize % align != 0 { - println!("[ferrisetw] buffer alignment mismatch"); return Err(ParserError::PropertyError( "buffer alignment mismatch".into() )); } if size.checked_mul(count).is_none() || (size * count) > isize::MAX as usize { - println!("[ferrisetw] size overflow"); return Err(ParserError::PropertyError( "size overflow".into() )); @@ -461,37 +462,23 @@ impl private::TryParse for Parser<'_, '_> { match prop_slice.property.info { PropertyInfo::Value { in_type, .. } => match in_type { TdhInType::InTypeUnicodeString => { - //let align = std::mem::align_of::(); - if prop_slice.buffer.len() % 2 != 0 { - println!("[ferrisetw] odd length in bytes for a wide string"); return Err(ParserError::PropertyError( "odd length in bytes for a wide string".into(), )); } + // std::slice::from_raw_parts requires a pointer to be aligned, but we can't + // guarantee that the buffer is aligned. In testing, I found that the buffer + // is in fact never aligned appropriately, so a cheap workaround is to copy + // the buffer into a new Vec and use that as the source for the slice + // until we can find a better solution. let mut aligned_buffer = Vec::with_capacity(prop_slice.buffer.len() / 2); for chunk in prop_slice.buffer.chunks_exact(2) { let part = u16::from_ne_bytes([chunk[0], chunk[1]]); aligned_buffer.push(part); } - - - // if prop_slice.buffer.as_ptr() as usize % align != 0 { - // println!("[ferrisetw] buffer alignment mismatch. align: {} size: {}", align, prop_slice.buffer.as_ptr() as usize); - // return Err(ParserError::PropertyError( - // "buffer alignment mismatch".into(), - // )); - // } - - // let mut wide = unsafe { - // std::slice::from_raw_parts( - // prop_slice.buffer.as_ptr() as *const u16, - // prop_slice.buffer.len() / 2, - // ) - // }; - let mut wide = aligned_buffer.as_slice(); match wide.last() {