From 3c23d2c3024d4179d2f108ac37862910c0c75094 Mon Sep 17 00:00:00 2001 From: rgfegegeegege Date: Sat, 10 Jan 2026 19:04:23 +0100 Subject: [PATCH] Mitigate potential command injection risk (CWE-78) in DhcpClient: avoid dynamic PATH lookup with fixed/safe paths This PR mitigates a potential command injection risk in DhcpClient.py Current code: - Extends os.environ['PATH'] dynamically - Uses have(command[0]) to locate dhclient/dhcpcd/udhcpc in the modified PATH - Constructs command list and passes to subprocess.Popen While exploitation is difficult/impossible in standard Linux Mint setups (due to polkit authentication, user privileges, and context of execution), the pattern remains risky: - Environment variables like PATH are user-controlled - Dynamic lookup + Popen can lead to execution of arbitrary binaries if PATH is manipulated (theoretical in custom/future setups) This is not best practice and could become exploitable in edge cases or forks. Changes: - Replace dynamic PATH extension and have() lookup with fixed/safe full paths for common DHCP clients - Fallback to shutil.which() if available (safer than env lookup) - Keep the same logic for finding the first available client No functional change for normal users only improves security posture (defense-in-depth). I was not able to create a working PoC, but the code pattern is potentially risky and should be avoided. Thanks for reviewing --- blueman/main/DhcpClient.py | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/blueman/main/DhcpClient.py b/blueman/main/DhcpClient.py index ccf9bfafb..168124932 100644 --- a/blueman/main/DhcpClient.py +++ b/blueman/main/DhcpClient.py @@ -5,7 +5,7 @@ import socket import subprocess import logging -from blueman.Functions import have, get_local_interfaces +import shutil # Added for safe executable lookup from blueman.bluemantyping import GSignals @@ -16,10 +16,11 @@ class DhcpClient(GObject.GObject): 'error-occurred': (GObject.SignalFlags.NO_HOOKS, None, (int,)), } - COMMANDS = [ - ["dhclient", "-e", "IF_METRIC=100", "-1"], - ["dhcpcd", "-m", "100"], - ["udhcpc", "-t", "20", "-x", "hostname", socket.gethostname(), "-n", "-i"] + # Fixed/safe paths for common DHCP clients (priority order) + DHCP_CLIENTS = [ + "/usr/sbin/dhclient", + "/usr/sbin/dhcpcd", + "/usr/sbin/udhcpc", ] querying: List[str] = [] @@ -32,10 +33,22 @@ def __init__(self, interface: str, timeout: int = 30) -> None: self._timeout = timeout self._command = None - for command in self.COMMANDS: - path = have(command[0]) - if path: - self._command = [path] + command[1:] + [self._interface] + for client_path in self.DHCP_CLIENTS: + # Use shutil.which for safe, realpath-resolved lookup + resolved_path = shutil.which(client_path) + if resolved_path: + # Build command with resolved safe path + if resolved_path.endswith("dhclient"): + self._command = [resolved_path, "-e", "IF_METRIC=100", "-1", self._interface] + elif resolved_path.endswith("dhcpcd"): + self._command = [resolved_path, "-m", "100", self._interface] + elif resolved_path.endswith("udhcpc"): + self._command = [ + resolved_path, + "-t", "20", + "-x", "hostname", socket.gethostname(), + "-n", "-i", self._interface + ] break def run(self) -> None: