Security: flash() keeps Tcl out of OpenOCD's program command (SDK 2.13.17)

packages/embeddedci/CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog — `embeddedci`
 
+## 2.13.17
+
+- Security: `flash()` puts the firmware path in braces in OpenOCD's `program` command and refuses a
+  `load_address` that is not a number, so a file name or address like `[exec ...]` can no longer
+  run a command through OpenOCD's Tcl. Windows paths are passed with forward slashes. A path
+  holding `{` or `}` is refused.
+
 ## 2.13.16
 
 - `open_i2c()` refused as busy names who holds the I2C master. When another connection has it

packages/embeddedci/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "embeddedci"
-version = "2.13.16"
+version = "2.13.17"
 description = "BenchPod SDK and pytest plugin for EmbeddedCI hardware-in-the-loop testing"
 readme = "README.md"
 requires-python = ">=3.10"

packages/embeddedci/src/embeddedci/benchpod/flash.py

@@ -16,6 +16,7 @@
 from __future__ import annotations
 
 import os
+import re
 import shutil
 import socket
 import subprocess
@@ -105,6 +106,19 @@ def supports_cmsis_dap_tcp(openocd_bin: str) -> bool:
         return False
 
 
+_LOAD_ADDR_RE = re.compile(r"^(0[xX][0-9a-fA-F]{1,8}|[0-9]{1,10})$")
+
+
+def _tcl_word(path: str) -> str:
+    """``path`` as one Tcl word with no substitution: in braces, which a path cannot break out of
+    unless it holds a brace (refused). Backslashes (Windows) become forward slashes, which OpenOCD
+    takes on Windows too, so none can escape a brace."""
+    path = path.replace("\\", "/")
+    if any(c in path for c in "{}\n\r"):
+        raise ValueError(f"the firmware path has characters OpenOCD would read as commands: {path!r}")
+    return "{" + path + "}"
+
+
 def build_openocd_args(
     target: str,
     file: str,
@@ -134,9 +148,14 @@ def build_openocd_args(
     if target and clear_reset_events:
         args += ["-c", _CLEAR_RESET_EVENTS_TCL]
     if file:
-        prog = ["program", file]
+        # The program command is Tcl: OpenOCD would run "[...]" in a file name or address as a
+        # command. The path goes in braces (no substitution inside) and the address must be a number.
+        prog = ["program", _tcl_word(file)]
         if load_address.strip():
-            prog.append(load_address.strip())
+            addr = load_address.strip()
+            if not _LOAD_ADDR_RE.match(addr):
+                raise ValueError(f"load_address must be a number like 0x08000000, not {load_address!r}")
+            prog.append(addr)
         if verify:
             prog.append("verify")
         if reset:

packages/embeddedci/tests/test_flash_bridge.py

@@ -137,4 +137,23 @@ def test_build_args_program_command():
     )
     assert "-f" in args and "target/stm32f1x.cfg" in args
     assert any("connect_assert_srst" in a for a in args)
-    assert any(a == "program fw.elf verify reset exit" for a in args)
+    assert any(a == "program {fw.elf} verify reset exit" for a in args)
+
+
+def test_build_args_keep_tcl_out_of_the_program_command():
+    def build(file, addr=""):
+        return flashmod.build_openocd_args(
+            "target/stm32f1x.cfg", file, addr,
+            verify=True, reset=True, connect_under_reset=False,
+            clear_reset_events=False, extra_configs=(), extra_args=(),
+        )
+
+    # "[...]" in a file name stays text inside braces; a Windows path keeps working
+    assert "program {/tmp/[exec id].elf} verify reset exit" in build("/tmp/[exec id].elf")
+    assert "program {C:/fw/app.bin} 0x08000000 verify reset exit" in build("C:\\fw\\app.bin", "0x08000000")
+    for bad_file in ("/tmp/a}b.elf", "/tmp/{x}.elf"):
+        with pytest.raises(ValueError):
+            build(bad_file)
+    for bad_addr in ("[exec id]", "0x0800 ; shutdown", "$x"):
+        with pytest.raises(ValueError):
+            build("fw.bin", bad_addr)