From c23c41a16105de1c0f9ee0dbfa88743c417496db Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 3 Apr 2026 15:03:42 -0700 Subject: [PATCH 1/3] Misc fixes for firewall and types --- .../src/types/fully_qualified_type_name.rs | 3 --- resources/windows_firewall/src/firewall.rs | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/dsc-lib/src/types/fully_qualified_type_name.rs b/lib/dsc-lib/src/types/fully_qualified_type_name.rs index 6e73a7f36..a4a2eb88e 100644 --- a/lib/dsc-lib/src/types/fully_qualified_type_name.rs +++ b/lib/dsc-lib/src/types/fully_qualified_type_name.rs @@ -1,9 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// TODO: Remove when updating to rustc 1.94; false positive from thiserror + rust-i18n t!() macro -#![allow(unused_assignments)] - use std::fmt::{Display, Formatter}; use std::hash::Hash; use std::ops::Deref; diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index bf75cd2aa..3a9ca93e3 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -69,6 +69,8 @@ impl FirewallStore { .cast() .map_err(|error| t!("firewall.ruleEnumerationFailed", error = error.to_string()).to_string())?; results.push(rule); + + unsafe { windows::Win32::System::Variant::VariantClear(&mut variant[0]) }; } Ok(results) @@ -169,6 +171,10 @@ fn profiles_from_mask(mask: i32) -> Vec { } fn profiles_to_mask(values: &[String]) -> Result { + if values.is_empty() { + return Ok(NET_FW_PROFILE2_ALL.0); + } + let mut mask = 0; for value in values { match value.to_ascii_lowercase().as_str() { @@ -197,6 +203,10 @@ fn join_csv(value: &[String]) -> String { } fn interface_types_to_string(values: &[String]) -> Result { + if values.is_empty() { + return Ok("All".to_string()); + } + let mut normalized = Vec::new(); for value in values { match value.to_ascii_lowercase().as_str() { @@ -269,6 +279,12 @@ fn apply_rule_properties(rule: &INetFwRule, desired: &FirewallRule, existing_pro // the existing rule's protocol (if updating an existing rule). let effective_protocol = desired.protocol.or(existing_protocol); + // If effective_protocol is None, read the current protocol from the rule. + let effective_protocol = match effective_protocol { + Some(protocol) => Some(protocol), + None => Some(unsafe { rule.Protocol() }.map_err(&err)?), + }; + // Reject port specifications for protocols that don't support them (e.g. ICMP). // This must be checked regardless of whether the protocol itself was changed, // because the caller may only be setting local_ports or remote_ports. From 524b47775210e34f94e3a55872674ae9911dadaf Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 6 Apr 2026 12:31:10 -0700 Subject: [PATCH 2/3] fix safety of VariantClear --- resources/windows_firewall/src/firewall.rs | 37 +++++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index 3a9ca93e3..80001f83e 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -8,11 +8,37 @@ use windows::Win32::Foundation::{S_FALSE, VARIANT_BOOL}; use windows::Win32::NetworkManagement::WindowsFirewall::*; use windows::Win32::System::Com::{CLSCTX_INPROC_SERVER, CoCreateInstance, CoInitializeEx, CoUninitialize, IDispatch, COINIT_APARTMENTTHREADED}; use windows::Win32::System::Ole::IEnumVARIANT; -use windows::Win32::System::Variant::VARIANT; +use windows::Win32::System::Variant::{VARIANT, VariantClear}; use crate::types::{FirewallError, FirewallRule, FirewallRuleList, RuleAction, RuleDirection}; use crate::util::matches_any_filter; +/// RAII wrapper for VARIANT that automatically calls VariantClear on drop +struct SafeVariant(VARIANT); + +impl SafeVariant { + fn new() -> Self { + Self(VARIANT::default()) + } + + fn as_mut_ptr(&mut self) -> *mut VARIANT { + &mut self.0 + } + + fn as_ref(&self) -> &VARIANT { + &self.0 + } +} + +impl Drop for SafeVariant { + fn drop(&mut self) { + let hr = unsafe { VariantClear(&mut self.0) }; + if hr.is_err() { + eprintln!("Warning: VariantClear failed with HRESULT: {:08x}", hr.0); + } + } +} + struct ComGuard; impl ComGuard { @@ -55,22 +81,23 @@ impl FirewallStore { let mut results = Vec::new(); loop { let mut fetched = 0u32; - let mut variant = [VARIANT::default()]; - let hr = unsafe { enum_variant.Next(&mut variant, &mut fetched) }; + let mut safe_variant = SafeVariant::new(); + let variant_slice = unsafe { std::slice::from_raw_parts_mut(safe_variant.as_mut_ptr(), 1) }; + let hr = unsafe { enum_variant.Next(variant_slice, &mut fetched) }; if hr == S_FALSE || fetched == 0 { break; } hr.ok() .map_err(|error| t!("firewall.ruleEnumerationFailed", error = error.to_string()).to_string())?; - let dispatch = IDispatch::try_from(&variant[0]) + let dispatch = IDispatch::try_from(safe_variant.as_ref()) .map_err(|error: windows::core::Error| t!("firewall.ruleEnumerationFailed", error = error.to_string()).to_string())?; let rule: INetFwRule = dispatch .cast() .map_err(|error| t!("firewall.ruleEnumerationFailed", error = error.to_string()).to_string())?; results.push(rule); - unsafe { windows::Win32::System::Variant::VariantClear(&mut variant[0]) }; + // SafeVariant will automatically call VariantClear when it goes out of scope } Ok(results) From 9044380e8f1462a6f08f18d018ad6a7cedb1d709 Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL HE/HIM) (from Dev Box)" Date: Mon, 6 Apr 2026 12:53:28 -0700 Subject: [PATCH 3/3] fix copilot feedback --- lib/dsc-lib/src/types/fully_qualified_type_name.rs | 3 +++ resources/windows_firewall/src/firewall.rs | 5 ++--- resources/windows_firewall/src/main.rs | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/dsc-lib/src/types/fully_qualified_type_name.rs b/lib/dsc-lib/src/types/fully_qualified_type_name.rs index a4a2eb88e..6e73a7f36 100644 --- a/lib/dsc-lib/src/types/fully_qualified_type_name.rs +++ b/lib/dsc-lib/src/types/fully_qualified_type_name.rs @@ -1,6 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +// TODO: Remove when updating to rustc 1.94; false positive from thiserror + rust-i18n t!() macro +#![allow(unused_assignments)] + use std::fmt::{Display, Formatter}; use std::hash::Hash; use std::ops::Deref; diff --git a/resources/windows_firewall/src/firewall.rs b/resources/windows_firewall/src/firewall.rs index 80001f83e..5c9556178 100644 --- a/resources/windows_firewall/src/firewall.rs +++ b/resources/windows_firewall/src/firewall.rs @@ -32,9 +32,8 @@ impl SafeVariant { impl Drop for SafeVariant { fn drop(&mut self) { - let hr = unsafe { VariantClear(&mut self.0) }; - if hr.is_err() { - eprintln!("Warning: VariantClear failed with HRESULT: {:08x}", hr.0); + if let Err(e) = unsafe { VariantClear(&mut self.0) } { + crate::write_error(&format!("Warning: VariantClear failed with HRESULT: {:#010x}", e.code().0)); } } } diff --git a/resources/windows_firewall/src/main.rs b/resources/windows_firewall/src/main.rs index c18093a95..3036ed2f1 100644 --- a/resources/windows_firewall/src/main.rs +++ b/resources/windows_firewall/src/main.rs @@ -19,7 +19,7 @@ const EXIT_INVALID_ARGS: i32 = 1; const EXIT_INVALID_INPUT: i32 = 2; const EXIT_FIREWALL_ERROR: i32 = 3; -fn write_error(message: &str) { +pub(crate) fn write_error(message: &str) { eprintln!("{}", serde_json::json!({ "error": message })); }