fix(security): prevent XML injection and remove dead code

Apply escape_xml_string() to all user-derived template substitutions
in XmlTemplate::apply_configuration() to prevent XML injection (CWE-91)
from crafted CSV input. Change apply_configuration signature from
&mut self to &self since it only reads base_content.

Extract network_base() helper in VlanConfig to eliminate 7x duplicated
strip_suffix pattern across gateway_ip(), dhcp_range_start(),
dhcp_range_end(), as_ipv4_network(), and static_reservations().

Remove no-op functions: setup_environment() in main.rs and
configure_terminal_with_global() in generate.rs. Remove unused
rand::Rng imports in nat.rs and vpn.rs (already covered by prelude).

Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>

Author: unclesp1d3r

benches/xml_generation.rs

@@ -20,7 +20,7 @@ fn bench_xml_generation(c: &mut Criterion) {
         let config = &configs[0];
 
         b.iter(|| {
-            let mut template = XmlTemplate::new(base_xml.to_string()).unwrap();
+            let template = XmlTemplate::new(base_xml.to_string()).unwrap();
             let result = template
                 .apply_configuration(black_box(config), 1, 6)
                 .unwrap();
@@ -37,7 +37,7 @@ fn bench_xml_generation(c: &mut Criterion) {
             b.iter(|| {
                 let mut results = Vec::new();
                 for config in &configs {
-                    let mut template = XmlTemplate::new(base_xml.to_string()).unwrap();
+                    let template = XmlTemplate::new(base_xml.to_string()).unwrap();
                     let result = template
                         .apply_configuration(black_box(config), 1, 6)
                         .unwrap();

mise.lock

@@ -32,3 +32,4 @@ lychee                      = "0.22.0"
 node                        = "25.4.0"
 zig                         = "0.15.2"
 python                      = "3.14.2"
+rust = "stable"

mise.toml

@@ -31,9 +31,6 @@ pub fn execute_with_global(mut args: GenerateArgs, global: &GlobalArgs) -> Resul
         }
     }
 
-    // Handle terminal compatibility
-    configure_terminal_with_global(&args, global);
-
     execute_internal(args, global)
 }
 
@@ -84,14 +81,6 @@ fn execute_internal(args: GenerateArgs, global: &GlobalArgs) -> Result<()> {
     }
 }
 
-/// Configure terminal output based on environment and arguments with global context
-fn configure_terminal_with_global(args: &GenerateArgs, global: &GlobalArgs) {
-    // Handle TERM=dumb compatibility - colors are automatically disabled
-    // by checking env::var("NO_COLOR").is_ok() and env::var("TERM") == "dumb"
-    // in the progress bar and console styling code
-    let _ = (args, global); // Suppress unused parameter warnings
-}
-
 /// Handle interactive mode prompts for missing required arguments
 fn handle_interactive_mode(mut args: GenerateArgs) -> Result<GenerateArgs> {
     let term = Term::stdout();
@@ -529,7 +518,7 @@ fn execute_xml_generation(args: &GenerateArgs, global: &GlobalArgs) -> Result<()
     // Load base XML template
     let base_xml = fs::read_to_string(base_config)
         .with_context(|| format!("Failed to read base config file: {:?}", base_config))?;
-    let mut template = XmlTemplate::new(base_xml)
+    let template = XmlTemplate::new(base_xml)
         .with_context(|| "Failed to create XML template from base configuration")?;
 
     // Set up progress for XML generation

src/cli/commands/generate.rs

@@ -63,7 +63,7 @@ pub fn execute(args: XmlArgs) -> Result<()> {
 
     // Load base XML template
     let base_xml = fs::read_to_string(&args.base_config)?;
-    let mut template = XmlTemplate::new(base_xml)?;
+    let template = XmlTemplate::new(base_xml)?;
 
     // Set up progress for XML generation
     let pb = ProgressBar::new(configs.len() as u64);

src/cli/commands/xml.rs

@@ -4,7 +4,6 @@
 //! mappings including port forwarding, source NAT, and destination NAT rules.
 
 use crate::model::ConfigError;
-use rand::Rng;
 use rand::prelude::*;
 use serde::{Deserialize, Serialize};
 use std::collections::HashSet;

src/generator/nat.rs

@@ -177,18 +177,17 @@ impl VlanConfig {
 
     /// Get the network as an Ipv4Network if possible
     pub fn as_ipv4_network(&self) -> VlanResult<Ipv4Network> {
-        if let Some(base) = self.ip_network.strip_suffix(".x") {
-            let network_str = format!("{base}.0/24");
-            rfc1918::validate_rfc1918_network_string(&network_str)
-        } else if let Some(base) = self.ip_network.strip_suffix(".0/24") {
-            let network_str = format!("{base}.0/24");
-            rfc1918::validate_rfc1918_network_string(&network_str)
-        } else {
-            Err(VlanError::network_parsing(format!(
-                "Cannot parse network format: {}",
-                self.ip_network
-            )))
-        }
+        let base = self
+            .ip_network
+            .strip_suffix(".x")
+            .or_else(|| self.ip_network.strip_suffix(".0/24"))
+            .ok_or_else(|| {
+                VlanError::network_parsing(format!(
+                    "Cannot parse network format: {}",
+                    self.ip_network
+                ))
+            })?;
+        rfc1918::validate_rfc1918_network_string(&format!("{base}.0/24"))
     }
 
     /// Validate that this configuration is RFC 1918 compliant
@@ -238,46 +237,34 @@ impl VlanConfig {
         "255.255.255.0"
     }
 
+    /// Extract the three-octet base prefix from the IP network string.
+    ///
+    /// Handles both "10.1.2.x" and "10.1.2.0/24" formats, returning "10.1.2".
+    fn network_base(&self) -> Result<&str> {
+        self.ip_network
+            .strip_suffix(".x")
+            .or_else(|| self.ip_network.strip_suffix(".0/24"))
+            .ok_or_else(|| {
+                ConfigError::validation(format!(
+                    "Cannot parse base from IP network: {}",
+                    self.ip_network
+                ))
+            })
+    }
+
     /// Get the gateway IP address (network + 1)
     pub fn gateway_ip(&self) -> Result<String> {
-        if let Some(base) = self.ip_network.strip_suffix(".x") {
-            Ok(format!("{base}.1"))
-        } else if let Some(base) = self.ip_network.strip_suffix(".0/24") {
-            Ok(format!("{base}.1"))
-        } else {
-            Err(ConfigError::validation(format!(
-                "Cannot derive gateway from IP network: {}",
-                self.ip_network
-            )))
-        }
+        Ok(format!("{}.1", self.network_base()?))
     }
 
     /// Get the DHCP range start IP
     pub fn dhcp_range_start(&self) -> Result<String> {
-        if let Some(base) = self.ip_network.strip_suffix(".x") {
-            Ok(format!("{base}.100"))
-        } else if let Some(base) = self.ip_network.strip_suffix(".0/24") {
-            Ok(format!("{base}.100"))
-        } else {
-            Err(ConfigError::validation(format!(
-                "Cannot derive DHCP range from IP network: {}",
-                self.ip_network
-            )))
-        }
+        Ok(format!("{}.100", self.network_base()?))
     }
 
     /// Get the DHCP range end IP
     pub fn dhcp_range_end(&self) -> Result<String> {
-        if let Some(base) = self.ip_network.strip_suffix(".x") {
-            Ok(format!("{base}.200"))
-        } else if let Some(base) = self.ip_network.strip_suffix(".0/24") {
-            Ok(format!("{base}.200"))
-        } else {
-            Err(ConfigError::validation(format!(
-                "Cannot derive DHCP range from IP network: {}",
-                self.ip_network
-            )))
-        }
+        Ok(format!("{}.200", self.network_base()?))
     }
 
     /// Get the DHCP lease time based on department type (in seconds)
@@ -345,16 +332,7 @@ impl VlanConfig {
         let mut reservations = Vec::new();
 
         // Get base network for IP assignments
-        let base = if let Some(base) = self.ip_network.strip_suffix(".x") {
-            base
-        } else if let Some(base) = self.ip_network.strip_suffix(".0/24") {
-            base
-        } else {
-            return Err(ConfigError::validation(format!(
-                "Cannot derive static reservations from IP network: {}",
-                self.ip_network
-            )));
-        };
+        let base = self.network_base()?;
 
         // Generate department-specific static reservations
         let department = self
@@ -1041,26 +1019,26 @@ mod tests {
         assert!(config.dhcp_range_start().is_err());
         assert!(config.dhcp_range_end().is_err());
 
-        // Test specific error messages
+        // All methods delegate to network_base(), which returns a unified error
         let gateway_error = config.gateway_ip().unwrap_err();
         assert!(
             gateway_error
                 .to_string()
-                .contains("Cannot derive gateway from IP network: invalid.network")
+                .contains("Cannot parse base from IP network: invalid.network")
         );
 
         let dhcp_start_error = config.dhcp_range_start().unwrap_err();
         assert!(
             dhcp_start_error
                 .to_string()
-                .contains("Cannot derive DHCP range from IP network: invalid.network")
+                .contains("Cannot parse base from IP network: invalid.network")
         );
 
         let dhcp_end_error = config.dhcp_range_end().unwrap_err();
         assert!(
             dhcp_end_error
                 .to_string()
-                .contains("Cannot derive DHCP range from IP network: invalid.network")
+                .contains("Cannot parse base from IP network: invalid.network")
         );
     }
 

src/generator/vlan.rs

@@ -4,7 +4,6 @@
 //! including OpenVPN, WireGuard, and IPSec tunnels for testing purposes.
 
 use crate::model::ConfigError;
-use rand::Rng;
 use rand::prelude::*;
 use serde::{Deserialize, Serialize};
 use std::collections::HashSet;

src/generator/vpn.rs

@@ -11,9 +11,6 @@ use opnsense_config_faker::cli::{Cli, Commands};
 fn main() -> Result<()> {
     let cli = Cli::parse();
 
-    // Set up environment with context
-    setup_environment(&cli).context("Failed to setup CLI environment")?;
-
     // Execute command with rich context
     match cli.command {
         Commands::Generate(args) => {
@@ -40,11 +37,3 @@ fn main() -> Result<()> {
 
     Ok(())
 }
-
-/// Set up the CLI environment with proper configuration
-fn setup_environment(cli: &Cli) -> Result<()> {
-    // Colors are automatically disabled by checking env::var("NO_COLOR").is_ok()
-    // and env::var("TERM") == "dumb" in the progress bar and console styling code
-    let _ = cli; // Suppress unused parameter warnings
-    Ok(())
-}

src/main.rs

@@ -26,7 +26,7 @@ impl XmlTemplate {
 
     /// Apply a VLAN configuration to generate an XML configuration
     pub fn apply_configuration(
-        &mut self,
+        &self,
         config: &VlanConfig,
         firewall_nr: u16,
         opt_counter: u16,
@@ -37,26 +37,27 @@ impl XmlTemplate {
 
         let mut result = self.base_content.clone();
 
-        // Replace placeholder values (simplified version)
+        // Replace placeholder values — all user-derived values are XML-escaped
+        // to prevent XML injection (CWE-91) from crafted CSV input
         result = result.replace("{{VLAN_ID}}", &config.vlan_id.to_string());
-        result = result.replace("{{IP_NETWORK}}", &config.ip_network);
-        result = result.replace("{{DESCRIPTION}}", &config.description);
+        result = result.replace("{{IP_NETWORK}}", &escape_xml_string(&config.ip_network));
+        result = result.replace("{{DESCRIPTION}}", &escape_xml_string(&config.description));
         result = result.replace("{{WAN_ASSIGNMENT}}", &config.wan_assignment.to_string());
         result = result.replace("{{FIREWALL_NR}}", &firewall_nr.to_string());
         result = result.replace("{{OPT_COUNTER}}", &opt_counter.to_string());
 
         // Add gateway IP if possible
         if let Ok(gateway) = config.gateway_ip() {
-            result = result.replace("{{GATEWAY_IP}}", &gateway);
+            result = result.replace("{{GATEWAY_IP}}", &escape_xml_string(&gateway));
         }
 
         // Add DHCP range if possible
         if let Ok(dhcp_start) = config.dhcp_range_start() {
-            result = result.replace("{{DHCP_START}}", &dhcp_start);
+            result = result.replace("{{DHCP_START}}", &escape_xml_string(&dhcp_start));
         }
 
         if let Ok(dhcp_end) = config.dhcp_range_end() {
-            result = result.replace("{{DHCP_END}}", &dhcp_end);
+            result = result.replace("{{DHCP_END}}", &escape_xml_string(&dhcp_end));
         }
 
         Ok(result)
@@ -112,7 +113,7 @@ mod tests {
     <gateway>{{GATEWAY_IP}}</gateway>
 </opnsense>"#;
 
-        let mut template = XmlTemplate::new(xml_content.to_string()).unwrap();
+        let template = XmlTemplate::new(xml_content.to_string()).unwrap();
         let config =
             VlanConfig::new(100, "10.1.2.x".to_string(), "Test VLAN 100".to_string(), 1).unwrap();