From 2e7fd3d65bff1f65e9d3092318edc9bbb9c1d346 Mon Sep 17 00:00:00 2001 From: Edward Viaene Date: Sat, 14 Nov 2026 14:24:40 -0600 Subject: [PATCH] Security: flash() keeps Tcl out of OpenOCD's program command (SDK 2.13.17) The firmware path goes in braces and load_address must be a number, so a file name or address like [exec ...] cannot run a command through OpenOCD's Tcl. Windows paths are passed with forward slashes; a path with a brace is refused. Co-Authored-By: Claude Opus 5.5 --- packages/embeddedci/CHANGELOG.md | 7 ++++++ packages/embeddedci/pyproject.toml | 2 +- .../src/embeddedci/benchpod/flash.py | 23 +++++++++++++++++-- .../embeddedci/tests/test_flash_bridge.py | 21 ++++++++++++++++- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/embeddedci/CHANGELOG.md b/packages/embeddedci/CHANGELOG.md index d4c31ce..604c8c9 100644 --- a/packages/embeddedci/CHANGELOG.md +++ b/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 diff --git a/packages/embeddedci/pyproject.toml b/packages/embeddedci/pyproject.toml index 7a02398..130cb8c 100644 --- a/packages/embeddedci/pyproject.toml +++ b/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" diff --git a/packages/embeddedci/src/embeddedci/benchpod/flash.py b/packages/embeddedci/src/embeddedci/benchpod/flash.py index 0082b12..63e67ba 100644 --- a/packages/embeddedci/src/embeddedci/benchpod/flash.py +++ b/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: diff --git a/packages/embeddedci/tests/test_flash_bridge.py b/packages/embeddedci/tests/test_flash_bridge.py index 32abb58..1fae02f 100644 --- a/packages/embeddedci/tests/test_flash_bridge.py +++ b/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)