From 034614fd1df1f6bb9e8ade02582e3dc795bbba8d Mon Sep 17 00:00:00 2001 From: sys-xmlcli Date: Fri, 25 Sep 2026 10:44:21 +0530 Subject: [PATCH 1/4] eliminate OS command injection via os.system --- src/xmlcli/access/winrwe/winrwe.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/xmlcli/access/winrwe/winrwe.py b/src/xmlcli/access/winrwe/winrwe.py index a890fcf..883dded 100644 --- a/src/xmlcli/access/winrwe/winrwe.py +++ b/src/xmlcli/access/winrwe/winrwe.py @@ -4,6 +4,7 @@ # Built-in imports import os import binascii +import subprocess # Custom imports from ..base import base @@ -33,22 +34,22 @@ def close_interface(self): return 0 def warm_reset(self): - os.system('{} /Nologo /Min /Command="O 0xCF9 0x06; RwExit"'.format(self.rw_executable)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0xCF9 0x06; RwExit"], shell=False) def cold_reset(self): - os.system('{} /Nologo /Min /Command="O 0xCF9 0x0E; RwExit"'.format(self.rw_executable)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0xCF9 0x0E; RwExit"], shell=False) def mem_block(self, address, size): - os.system('{} /Nologo /Min /Command="SAVE {} Memory 0x{:x} 0x{:x}; RwExit"'.format(self.rw_executable, self.temp_data_bin, address, size)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(self.temp_data_bin, address, size)], shell=False) with open(self.temp_data_bin, 'rb') as f: data_buffer = f.read() return data_buffer def mem_save(self, filename, address, size): - os.system('{} /Nologo /Min /Command="SAVE {} Memory 0x{:x} 0x{:x}; RwExit"'.format(self.rw_executable, filename, address, size)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(filename, address, size)], shell=False) def mem_read(self, address, size): - os.system('{} /Nologo /Min /Command="SAVE {} Memory 0x{:x} 0x{:x}; RwExit"'.format(self.rw_executable, self.temp_data_bin, address, size)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(self.temp_data_bin, address, size)], shell=False) with open(self.temp_data_bin, 'rb') as f: data_buffer = f.read() return int(binascii.hexlify(data_buffer[0:size][::-1]), 16) @@ -60,15 +61,15 @@ def mem_write(self, address, size, value): cmd = "W{} 0x{:x} 0x{:x}".format(word_size, address, value) else: cmd = "W{} 0x{:x} 0x{:x}; W32 0x{:x} 0x{:x}".format(32, address, (value & 0xFFFFFFFF), (address + 4), (value >> 32)) - os.system('{} /Nologo /Min /Command="{}; RwExit"'.format(self.rw_executable, cmd)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command={}; RwExit".format(cmd)], shell=False) def load_data(self, filename, address): - os.system('{} /Nologo /Min /Command="LOAD {} Memory 0x{:x}; RwExit"'.format(self.rw_executable, filename, address)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=LOAD {} Memory 0x{:x}; RwExit".format(filename, address)], shell=False) def read_io(self, address, size): if size in (1, 2, 4): cmd = "I{} 0x{:x}".format("" if size == 1 else 8*size, address) - os.system('{} /Nologo /Min /LogFile={} /Command="{}; RwExit"'.format(self.rw_executable, self.result_text, cmd)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/LogFile={}".format(self.result_text), "/Command={}; RwExit".format(cmd)], shell=False) with open(self.result_text, 'r') as f: result = f.read() temp_str = result.split('=') @@ -80,10 +81,10 @@ def read_io(self, address, size): def write_io(self, address, size, value): if size in (1, 2, 4): cmd = "O{} 0x{:x} 0x{:x}".format("" if size == 1 else 8*size, address, value) - os.system('{} /Nologo /Min /Command="{}; RwExit"'.format(self.rw_executable, cmd)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command={}; RwExit".format(cmd)], shell=False) def trigger_smi(self, smi_value): - os.system('{} /Nologo /Min /Command="O 0x{:x} 0x{:x}; RwExit"'.format(self.rw_executable, 0xB2, smi_value)) + subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0x{:x} 0x{:x}; RwExit".format(0xB2, smi_value)], shell=False) def read_msr(self, Ap, address): return 0 From 5232ed799e8125295c7f06fa72587393b728229c Mon Sep 17 00:00:00 2001 From: prakashb72 Date: Wed, 7 Oct 2026 11:19:04 +0530 Subject: [PATCH 2/4] Update winrwe.py --- src/xmlcli/access/winrwe/winrwe.py | 45 ++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/xmlcli/access/winrwe/winrwe.py b/src/xmlcli/access/winrwe/winrwe.py index 883dded..7c75d39 100644 --- a/src/xmlcli/access/winrwe/winrwe.py +++ b/src/xmlcli/access/winrwe/winrwe.py @@ -3,6 +3,7 @@ # Built-in imports import os +import shutil import binascii import subprocess @@ -12,14 +13,30 @@ __all__ = ["WinRweAccess"] +# RW.exe parses its own /Command= mini-language: `;` separates statements and whitespace separates tokens. +_RW_COMMAND_UNSAFE_CHARS = ";\"' \t\r\n" + class WinRweAccess(base.BaseAccess): def __init__(self, access_name="winrwe"): self.current_directory = os.path.dirname(os.path.abspath(__file__)) super(WinRweAccess, self).__init__(access_name=access_name, child_class_directory=self.current_directory) self.rw_executable = self.config.get(access_name.upper(), "RW_EXE") - self.temp_data_bin = self.config.get(access_name.upper(), "TEMP_DATA_BIN") - self.result_text = self.config.get(access_name.upper(), "RESULT_TEXT") + self.temp_data_bin = self._validate_rw_path(self.config.get(access_name.upper(), "TEMP_DATA_BIN"), "TEMP_DATA_BIN") + self.result_text = self._validate_rw_path(self.config.get(access_name.upper(), "RESULT_TEXT"), "RESULT_TEXT") + + @staticmethod + def _validate_rw_path(path, setting_name): + if not path or any(character in path for character in _RW_COMMAND_UNSAFE_CHARS): + raise ValueError("{} must be a non-empty path without whitespace, quotes or ';' (got {!r})".format(setting_name, path)) + return path + + def _run_rw(self, command, log_file=None): + arguments = [self.rw_executable, "/Nologo", "/Min"] + if log_file: + arguments.append("/LogFile={}".format(log_file)) + arguments.append("/Command={}; RwExit".format(command)) + return subprocess.run(arguments, shell=False) # nosec B603 - fixed argv, no shell, no caller-controlled tokens def halt_cpu(self, delay=0): return 0 @@ -34,22 +51,24 @@ def close_interface(self): return 0 def warm_reset(self): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0xCF9 0x06; RwExit"], shell=False) + self._run_rw("O 0xCF9 0x06") def cold_reset(self): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0xCF9 0x0E; RwExit"], shell=False) + self._run_rw("O 0xCF9 0x0E") def mem_block(self, address, size): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(self.temp_data_bin, address, size)], shell=False) + self._run_rw("SAVE {} Memory 0x{:x} 0x{:x}".format(self.temp_data_bin, address, size)) with open(self.temp_data_bin, 'rb') as f: data_buffer = f.read() return data_buffer def mem_save(self, filename, address, size): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(filename, address, size)], shell=False) + # RW only ever writes to the internal temp path, so `filename` never reaches its command parser. + self._run_rw("SAVE {} Memory 0x{:x} 0x{:x}".format(self.temp_data_bin, address, size)) + shutil.copyfile(self.temp_data_bin, filename) def mem_read(self, address, size): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=SAVE {} Memory 0x{:x} 0x{:x}; RwExit".format(self.temp_data_bin, address, size)], shell=False) + self._run_rw("SAVE {} Memory 0x{:x} 0x{:x}".format(self.temp_data_bin, address, size)) with open(self.temp_data_bin, 'rb') as f: data_buffer = f.read() return int(binascii.hexlify(data_buffer[0:size][::-1]), 16) @@ -61,15 +80,17 @@ def mem_write(self, address, size, value): cmd = "W{} 0x{:x} 0x{:x}".format(word_size, address, value) else: cmd = "W{} 0x{:x} 0x{:x}; W32 0x{:x} 0x{:x}".format(32, address, (value & 0xFFFFFFFF), (address + 4), (value >> 32)) - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command={}; RwExit".format(cmd)], shell=False) + self._run_rw(cmd) def load_data(self, filename, address): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=LOAD {} Memory 0x{:x}; RwExit".format(filename, address)], shell=False) + # Stage the caller's file through the internal temp path so `filename` never reaches RW's command parser. + shutil.copyfile(filename, self.temp_data_bin) + self._run_rw("LOAD {} Memory 0x{:x}".format(self.temp_data_bin, address)) def read_io(self, address, size): if size in (1, 2, 4): cmd = "I{} 0x{:x}".format("" if size == 1 else 8*size, address) - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/LogFile={}".format(self.result_text), "/Command={}; RwExit".format(cmd)], shell=False) + self._run_rw(cmd, log_file=self.result_text) with open(self.result_text, 'r') as f: result = f.read() temp_str = result.split('=') @@ -81,10 +102,10 @@ def read_io(self, address, size): def write_io(self, address, size, value): if size in (1, 2, 4): cmd = "O{} 0x{:x} 0x{:x}".format("" if size == 1 else 8*size, address, value) - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command={}; RwExit".format(cmd)], shell=False) + self._run_rw(cmd) def trigger_smi(self, smi_value): - subprocess.run([self.rw_executable, "/Nologo", "/Min", "/Command=O 0x{:x} 0x{:x}; RwExit".format(0xB2, smi_value)], shell=False) + self._run_rw("O 0x{:x} 0x{:x}".format(0xB2, smi_value)) def read_msr(self, Ap, address): return 0 From e4541e09b57263668355d5e132a05f3b3fa92b45 Mon Sep 17 00:00:00 2001 From: prakashb72 Date: Wed, 7 Oct 2026 11:21:41 +0530 Subject: [PATCH 3/4] Update pyproject.toml --- pyproject.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 3e3d477..71d553c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,12 @@ Repository = "https://github.com/intel/xml-cli" xmlcli = "xmlcli.start_xmlcli:cli" uefi-analyze = "xmlcli.modules.uefi_analyzer.cli:main" + [tool.poetry] +name = "xmlcli" +version = "2.0.6" +description = "UFFAF - UEFI Firmware Foundational Automation Framework (formerly Xml-Cli)" +authors = ["Gahan Saraya "] packages = [{include = "xmlcli", from = "src"}] [tool.poetry.group.dev.dependencies] From 54f4f3722f29e891139da174529e179a7341552b Mon Sep 17 00:00:00 2001 From: prakashb72 Date: Wed, 7 Oct 2026 11:25:19 +0530 Subject: [PATCH 4/4] Update winrwe.py --- src/xmlcli/access/winrwe/winrwe.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/xmlcli/access/winrwe/winrwe.py b/src/xmlcli/access/winrwe/winrwe.py index 7c75d39..e8a4a45 100644 --- a/src/xmlcli/access/winrwe/winrwe.py +++ b/src/xmlcli/access/winrwe/winrwe.py @@ -31,6 +31,12 @@ def _validate_rw_path(path, setting_name): raise ValueError("{} must be a non-empty path without whitespace, quotes or ';' (got {!r})".format(setting_name, path)) return path + @staticmethod + def _copy_unless_same(source, destination): + if os.path.exists(destination) and os.path.samefile(source, destination): + return + shutil.copyfile(source, destination) + def _run_rw(self, command, log_file=None): arguments = [self.rw_executable, "/Nologo", "/Min"] if log_file: @@ -65,7 +71,7 @@ def mem_block(self, address, size): def mem_save(self, filename, address, size): # RW only ever writes to the internal temp path, so `filename` never reaches its command parser. self._run_rw("SAVE {} Memory 0x{:x} 0x{:x}".format(self.temp_data_bin, address, size)) - shutil.copyfile(self.temp_data_bin, filename) + self._copy_unless_same(self.temp_data_bin, filename) def mem_read(self, address, size): self._run_rw("SAVE {} Memory 0x{:x} 0x{:x}".format(self.temp_data_bin, address, size)) @@ -84,7 +90,7 @@ def mem_write(self, address, size, value): def load_data(self, filename, address): # Stage the caller's file through the internal temp path so `filename` never reaches RW's command parser. - shutil.copyfile(filename, self.temp_data_bin) + self._copy_unless_same(filename, self.temp_data_bin) self._run_rw("LOAD {} Memory 0x{:x}".format(self.temp_data_bin, address)) def read_io(self, address, size):