From 8f9959069554446ed921103dc7421ca3256c32fc Mon Sep 17 00:00:00 2001 From: Prekzursil Date: Tue, 11 Aug 2026 07:31:40 +0300 Subject: [PATCH] feat(quality): add curated .quality/opengrep ruleset (arms lean gate 4) This repo shipped no `.quality/opengrep`, so the lean gate's SAST lane (gate 4 of reusable-quality.yml) was skipped entirely and the `quality` job still reported success -- a skip-and-pass. This adds the missing pinned, in-repo ruleset so gate 4 actually runs. ## What lands -- 19 rules across the 3 languages opengrep can see here - `.quality/opengrep/csharp-security.yaml` (6) -- `BinaryFormatter`/ `SoapFormatter`/`NetDataContractSerializer`/`LosFormatter`; Newtonsoft `TypeNameHandling != None`; a TLS validation callback returning `true` unconditionally; MD5/SHA-1; `Process.Start` with a runtime-built command string; `Assembly.Load*` from a computed path. - `.quality/opengrep/python-security.yaml` (8) -- `exec`, `eval`, `subprocess(..., shell=True)`, `os.system`/`os.popen`, unsafe `yaml.load`, `pickle`/`marshal` loads, md5/sha1, TLS verification disabled. - `.quality/opengrep/cpp-security.yaml` (3) -- unbounded `strcpy`/`strcat`/ `sprintf`/`gets` with a **non-literal** source, `system`/`popen`/`_wsystem`, `tmpnam`/`mktemp`/`tempnam`. - `.quality/opengrep/general-security.yaml` (2) -- committed PEM private key, AWS access key id. - `.quality/opengrep/README.md` -- language inventory, the vendored-tree exclusions with their upstream licences, the suppression table, the measured coverage residuals, and the both-states proof procedure. - One inline `# nosemgrep:` suppression in `swfoc_toolkit/live_test.py` (comments only, no behaviour change). Scope per language was measured, not guessed: C# 398 files (`src/` 192, `tests/` 206), Python 60, C/C++ 108, PowerShell 48, Lua 5. **PowerShell gets no lane** -- opengrep 1.22.0 does not support the language (`opengrep show supported-languages`). **Lua gets no lane** -- there is no `load`/`loadstring`/`dofile`/`os.execute`/`io.popen` in the 5 `.lua` files, so a rule there would be decoration. Both are stated in the README rather than left to look covered. ## Why curated + pinned, not `--config auto` A registry-fetched ruleset changes underneath you: green today, red tomorrow with no change of yours. That externally-refreshing finding set is the treadmill the lean charter exists to escape. A fixed in-repo ruleset makes the gate deterministic, so zero is both reachable and stable. ## Verification (measured 2026-08-11, opengrep 1.22.0 -- the CI pin) Both states were checked; a ruleset that has never gone red is indistinguishable from an empty file. The fired-rule set is taken from `--json` `results[].check_id`, because the pretty console output wraps long rule ids and under-reports which rules fired. - KNOWN-BAD control (`Vuln.cs` + `vuln.py` + `vuln.cpp` + a fake PEM/AKIA fixture, outside the repo): **19/19 declared rules fired, 32 findings, exit 1, 0 engine errors**. - KNOWN-GOOD control (safe equivalents -- `System.Text.Json`, `TypeNameHandling.None`, SHA-256, `ProcessStartInfo` + `ArgumentList`, `ast.literal_eval`, `shell=False`, `yaml.safe_load`, `snprintf`): **0 findings, exit 0, 0 engine errors**. - REAL repo, gate-4 invocation replayed verbatim on a native Linux filesystem: **`Ran 19 rules on 46724 files: 1 finding`, exit 1** before the suppression; **0 findings, exit 0** after. Wall clock ~140-210 s. - gitleaks 8.30.1 (gate 5) with the new files present: exit 0; detector validated -- it exits 1 on the known-bad fixture. ## The 1 real finding, and why it is annotated rather than hidden `swfoc_toolkit/live_test.py` `launch_game()` -- `subprocess.Popen(["cmd", "/c", f"start steam://rungameid/{STEAM_APP_ID}"], shell=True)`. The argv is a fixed literal list whose only interpolation is the module-level `STEAM_APP_ID` constant, so **no untrusted input reaches the shell** -- it is not a command-injection vulnerability. `shell=True` is nonetheless redundant here (the list already invokes `cmd /c`) and should be dropped; that is recorded as a follow-up rather than changed inside a ruleset-adoption PR, because this harness drives a real game process and cannot be exercised in CI. The suppression is inline and greppable (`grep -rn nosemgrep`), and the reason is in the code and in the README table. No `paths: exclude` entry hides it. Note the Python rules are deliberately **wider** than `ruff.toml`'s `extend-exclude`: a security lane should not inherit a style lane's scope, so `swfoc_toolkit/`, `swfoc_lua_bridge/`, `bridge/` and `ghidra_scripts/` are scanned. ## Vendored trees excluded from the C/C++ rules (named, with licences) `swfoc_overlay/imgui/**` (Dear ImGui, MIT, (c) 2014-2024 Omar Cornut) and `swfoc_lua_bridge/minhook/**` (MinHook, (c) 2009-2017 Tsuda Kageyu), both carrying their upstream `LICENSE.txt` in-tree. This mirrors the DevExtreme ruleset's `vendor/` exclusion and is not hiding a first-party hit: with the exclusions removed the additional match count is still zero, because every `strcpy` in this repo has a string-literal source and the rule already exempts that shape. ## Gate-authoring traps this PR documents (README) 1. opengrep exits **2** on a rule parse error while still printing `0 findings` -- a broken rule reads as "clean" if you only look at the count, and reds the gate permanently in CI. Check the exit code. 2. opengrep reports rule **timeouts**, **partial parses** and **too-big skips** as `level: warn` and **still exits 0** -- so gate 4 can silently stop covering a file while staying green. The README enumerates the four measured residuals on this repo (78 `knowledge-base/` files >1 MB; a Memprof limit on `lua_bridge.cpp`; PartialParsing in 4 source files; an intermittent, purely load-dependent 5 s rule timeout on the 230 KB `RuntimeAdapter.cs` -- 0/5 timeouts when that file is scanned alone). The `Process.Start` rule was rewritten from a `metavariable-pattern` to a flat `pattern-either` to cut that cost. 3. `nosemgrep` is only honoured on the **same line as the finding or the line immediately above it**; a reason block ending several lines above suppresses nothing. --- .quality/opengrep/README.md | 150 ++++++++++++++++++++++++ .quality/opengrep/cpp-security.yaml | 79 +++++++++++++ .quality/opengrep/csharp-security.yaml | 118 +++++++++++++++++++ .quality/opengrep/general-security.yaml | 42 +++++++ .quality/opengrep/python-security.yaml | 119 +++++++++++++++++++ swfoc_toolkit/live_test.py | 7 ++ 6 files changed, 515 insertions(+) create mode 100644 .quality/opengrep/README.md create mode 100644 .quality/opengrep/cpp-security.yaml create mode 100644 .quality/opengrep/csharp-security.yaml create mode 100644 .quality/opengrep/general-security.yaml create mode 100644 .quality/opengrep/python-security.yaml diff --git a/.quality/opengrep/README.md b/.quality/opengrep/README.md new file mode 100644 index 000000000..5c8706178 --- /dev/null +++ b/.quality/opengrep/README.md @@ -0,0 +1,150 @@ +# Curated SAST ruleset (lean gate 4) + +Pinned tool: **opengrep 1.22.0** (CI, installed by +`Prekzursil/quality-zero-platform/.github/workflows/reusable-quality.yml`) — +locally interchangeable with **semgrep CE** (opengrep is a fork of semgrep and +consumes the same rule syntax). + +## Why an in-repo ruleset instead of `--config auto` + +`--config auto` / `p/*` registry packs are fetched from the network at scan time +and change underneath you: a repo that is green today goes red tomorrow with no +change of yours. That externally-refreshing finding-set is exactly the treadmill +the lean charter exists to escape. A fixed, reviewable ruleset committed to the +repo makes the gate deterministic — same result every run, offline, no registry +login — so **zero is both reachable and stable**. + +## Languages this repo actually contains + +Measured (`git ls-files` over the tracked tree, and +`gh api repos/Prekzursil/SWFOC-Mod-Menu/languages`): + +| language | files | where | rules | +| ---------- | ----- | ---------------------------------------------------------- | ----- | +| C# | 398 | `src/` (192), `tests/` (206) | 6 | +| Python | 60 | `tests/` (19), `swfoc_toolkit/` (17), `tools/` (10), `scripts/` (7), `swfoc_lua_bridge/` (3), `bridge/` (3) | 8 | +| C / C++ | 108 | `swfoc_overlay/`, `native/`, `swfoc_lua_bridge/` | 3 | +| PowerShell | 48 | `tools/`, `bridge/` | 0 — opengrep 1.22.0 does not support PowerShell (`opengrep show supported-languages`); those scripts are out of reach of gate 4 and are noted as a residual, not silently implied covered. | +| Lua | 5 | `profiles/`, `mods/`, `bridge/` | 0 — no `load`/`loadstring`/`dofile`/`os.execute`/`io.popen` exists in them today; a rule with no reachable sink would be decoration. | + +`knowledge-base/` (46,165 files, almost all machine-generated RE decompile/data +JSON) carries no code and is scanned only by the 2 language-agnostic secret +rules. It is **not** excluded. + +## Contents — 19 rules + +- `csharp-security.yaml` — `BinaryFormatter`/`SoapFormatter`/ + `NetDataContractSerializer`/`LosFormatter`; Newtonsoft + `TypeNameHandling != None`; a TLS validation callback that returns `true` + unconditionally; MD5/SHA-1; `Process.Start` with a runtime-built command + string (interpolation / concatenation / `string.Format`); `Assembly.Load*` + from a computed path. +- `python-security.yaml` — `exec`, `eval`, `subprocess(..., shell=True)`, + `os.system`/`os.popen`, unsafe `yaml.load`, `pickle`/`marshal` loads, + md5/sha1, TLS verification disabled. +- `cpp-security.yaml` — unbounded `strcpy`/`strcat`/`sprintf`/`gets` with a + **non-literal** source, `system`/`popen`/`_wsystem`, + `tmpnam`/`mktemp`/`tempnam`. +- `general-security.yaml` — committed PEM private key, AWS access key id + (defence-in-depth alongside gate 5 / gitleaks, a different engine). + +### Vendored third-party trees excluded from the C/C++ rules + +Named explicitly, and only on the C/C++ rules (mirrors the DevExtreme ruleset's +`vendor/` exclusion): + +| path | upstream | +| ----------------------------- | --------------------------------------------------- | +| `swfoc_overlay/imgui/**` | Dear ImGui — MIT, (c) 2014-2024 Omar Cornut | +| `swfoc_lua_bridge/minhook/**` | MinHook — (c) 2009-2017 Tsuda Kageyu | + +Both carry their upstream `LICENSE.txt` in-tree. This is not used to hide a +first-party hit: with the exclusions removed, the only additional C/C++ matches +would still be zero, because every `strcpy` in this repo (vendored or not) has a +**string-literal** source and the rule already exempts that shape. + +The Python rules are deliberately **wider** than `ruff.toml`'s +`extend-exclude`: a security lane should not inherit a style lane's scope, so +`swfoc_toolkit/`, `swfoc_lua_bridge/`, `bridge/` and `ghidra_scripts/` **are** +scanned here. + +## Running the gate + +```bash +# CI (opengrep, exactly as reusable-quality.yml gate 4 invokes it): +opengrep scan --config .quality/opengrep --error \ + --exclude .venv --exclude node_modules --exclude dist --exclude out --exclude build . + +# Local (semgrep CE, rule-compatible): +semgrep scan --config .quality/opengrep --error --metrics off \ + --exclude .venv --exclude node_modules --exclude dist --exclude out --exclude build . +``` + +Gate passes on **0 findings** (clean-zero lock; no baseline file). +Runtime measured on a native Linux filesystem: **~140-210 s for 46,724 files**. +(On a Windows `/mnt/c` 9p mount the same scan is I/O-bound and did not finish in +26 minutes — do not benchmark it there.) + +## Known coverage residuals — disclosed, not implied away + +opengrep reports these as `level: warn`. **They do not change the exit code**, so +gate 4 stays green while a rule has silently stopped covering a file. Measured +on this repo, 2026-08-11: + +| residual | effect | +| --------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | +| `Scan skipped: 78 files larger than 1.0 MB` — all in `knowledge-base/` | the 2 generic secret rules do not read them. Gate 5 (gitleaks) still scans them with a different engine. | +| `swfoc_lua_bridge/lua_bridge.cpp` → `Memprof_limits.Limit_reached` | the 3 C/C++ rules do not fully analyse the largest native file (~9.9k lines). | +| PartialParsing in `swfoc_overlay/overlay.cpp`, `swfoc_lua_bridge/proxy.cpp`, `src/SwfocTrainer.App/ViewModels/MainViewModel.cs`, `src/SwfocTrainer.Runtime/Services/BackendRouter.cs` | opengrep's parser cannot fully parse a construct in each; the rest of each file is still analysed. | +| An intermittent 5 s rule timeout on `src/SwfocTrainer.Runtime/Services/RuntimeAdapter.cs` (6,187 lines / 230 KB) | load-dependent, not deterministic: 0/5 timeouts when that file is scanned alone (9.4-33 s wall), but it appeared in 2 of 3 whole-repo runs on a loaded machine. It can only *reduce* coverage, never red the gate. | + +PowerShell (48 `.ps1` files) has no lane at all — opengrep 1.22.0 does not +support the language. + +## Suppressions + +Inline, greppable, one per site (`grep -rn nosemgrep`). No `paths: exclude` +entry narrows a rule to hide a first-party hit. + +| site | rule | why | +| --------------------------- | ------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `swfoc_toolkit/live_test.py` `launch_game()` | `python-subprocess-shell-true` | The argv is a fixed literal list whose only interpolation is the module-level `STEAM_APP_ID` constant, so no untrusted input reaches the shell. **`shell=True` is nonetheless redundant** (the list already invokes `cmd /c`) and should be dropped — recorded as a follow-up, since this harness drives a real game process and cannot be exercised in CI. | + +semgrep/opengrep only honour a `nosemgrep` marker on the **same line as the +finding or the line immediately above it** — a reason block ending 4 lines above +does not suppress anything. + +## Refreshing against upstream + +Upstream registry rules (`p/csharp`, `p/python`, `p/c`, `p/r2c-security-audit`) +are Apache-2.0 / LGPL-2.1; rule logic is reproduced / adapted here. To refresh, +diff the registry packs and port new high-signal rules in one-in-one-out, +re-running the control fixtures below. + +## Proving the ruleset can fire (do this on every change) + +A ruleset that has never gone red is indistinguishable from an empty file. Both +states must be checked, and the KNOWN-GOOD run's **exit code** matters as much +as its finding count: opengrep exits **2** on a rule parse error while still +printing `0 findings`, so a broken rule reads as "clean" locally and reds the +gate permanently in CI. + +```bash +# KNOWN-BAD: must exit 1, and every declared rule id must appear +opengrep scan --config .quality/opengrep --error --json --output bad.json fixtures/bad/ +# KNOWN-GOOD: must exit 0, 0 findings, and 0 entries in .errors +opengrep scan --config .quality/opengrep --error --json --output good.json fixtures/good/ +``` + +Take the fired-rule set from the **JSON** `results[].check_id`, not from the +pretty console output — the console wraps long rule ids and under-reports which +rules fired. + +Measured 2026-08-11 with opengrep 1.22.0: 19/19 declared rules fired on the +known-bad fixtures (32 findings, exit 1); known-good fixtures 0 findings, exit 0, +0 engine errors; this repo `Ran 19 rules on 46724 files: 1 finding` before the +suppression above and **0 findings, exit 0** after. + +Also check the warning lines (`timeout error`, `Partially scanned`, +`Scan skipped`) on every change — they are the only signal that a rule stopped +covering a file, and they never affect the exit code. diff --git a/.quality/opengrep/cpp-security.yaml b/.quality/opengrep/cpp-security.yaml new file mode 100644 index 000000000..455d48a6b --- /dev/null +++ b/.quality/opengrep/cpp-security.yaml @@ -0,0 +1,79 @@ +# Curated C / C++ security rules for the lean gate-4 (opengrep / Semgrep CE). +# Covers this repo's FIRST-PARTY native code: swfoc_lua_bridge/ (the injected +# Lua bridge DLL), swfoc_overlay/ (the ImGui overlay) and native/. +# +# VENDORED third-party trees are excluded per-rule and are named explicitly: +# swfoc_overlay/imgui/** Dear ImGui (MIT, (c) 2014-2024 Omar Cornut) +# swfoc_lua_bridge/minhook/** MinHook ((c) 2009-2017 Tsuda Kageyu) +# Both carry their upstream LICENSE.txt in-tree. This mirrors the DevExtreme +# ruleset's `vendor/` exclusion; it is not used to hide a first-party hit. +rules: + - id: cpp-unsafe-unbounded-string-copy + languages: [c, cpp] + severity: ERROR + message: >- + Unbounded string operation with a non-literal source (strcpy/strcat/ + sprintf/gets/scanf %s). The destination size is not consulted, so a longer + source overflows it. Use snprintf / strncpy+explicit NUL / std::string. + metadata: + category: security + cwe: "CWE-120: Buffer Copy without Checking Size of Input" + paths: + exclude: + - "swfoc_overlay/imgui/**" + - "swfoc_lua_bridge/minhook/**" + patterns: + - pattern-either: + - pattern: strcpy($DST, $SRC) + - pattern: std::strcpy($DST, $SRC) + - pattern: strcat($DST, $SRC) + - pattern: std::strcat($DST, $SRC) + - pattern: sprintf($DST, $SRC, ...) + - pattern: gets($DST) + - pattern-not: strcpy($DST, "...") + - pattern-not: std::strcpy($DST, "...") + - pattern-not: strcat($DST, "...") + - pattern-not: std::strcat($DST, "...") + + - id: cpp-shell-command-execution + languages: [c, cpp] + severity: ERROR + message: >- + system()/popen()/_wsystem() spawns a shell. In an injected in-process DLL + this is both a command-injection sink and a red flag for AV. Use + CreateProcess with an explicit argument vector. + metadata: + category: security + cwe: "CWE-78: OS Command Injection" + paths: + exclude: + - "swfoc_overlay/imgui/**" + - "swfoc_lua_bridge/minhook/**" + patterns: + - pattern-either: + - pattern: system($CMD) + - pattern: std::system($CMD) + - pattern: popen($CMD, ...) + - pattern: _popen($CMD, ...) + - pattern: _wsystem($CMD) + + - id: cpp-insecure-temp-file + languages: [c, cpp] + severity: ERROR + message: >- + tmpnam/mktemp/tempnam return a name, not an open handle, so the file can + be replaced between the name being chosen and the open (TOCTOU symlink + attack). Use mkstemp / GetTempFileName + exclusive create. + metadata: + category: security + cwe: "CWE-377: Insecure Temporary File" + paths: + exclude: + - "swfoc_overlay/imgui/**" + - "swfoc_lua_bridge/minhook/**" + patterns: + - pattern-either: + - pattern: tmpnam(...) + - pattern: std::tmpnam(...) + - pattern: mktemp(...) + - pattern: tempnam(...) diff --git a/.quality/opengrep/csharp-security.yaml b/.quality/opengrep/csharp-security.yaml new file mode 100644 index 000000000..4cb9e70ba --- /dev/null +++ b/.quality/opengrep/csharp-security.yaml @@ -0,0 +1,118 @@ +# Curated C# security rules for the lean gate-4 (opengrep / Semgrep CE). +# Scoped to this repo's .NET surface: src/ (Runtime/Catalog/UI) + tests/. +# See README.md. +rules: + - id: csharp-binaryformatter-deserialize + languages: [csharp] + severity: ERROR + message: >- + BinaryFormatter/SoapFormatter/NetDataContractSerializer deserialization is + insecure by design and cannot be made safe (it is obsolete and removed in + .NET 9). Use System.Text.Json or a contract-bound serializer. + metadata: + category: security + cwe: "CWE-502: Deserialization of Untrusted Data" + patterns: + - pattern-either: + - pattern: new BinaryFormatter(...) + - pattern: new SoapFormatter(...) + - pattern: new NetDataContractSerializer(...) + - pattern: new LosFormatter(...) + + - id: csharp-json-typenamehandling + languages: [csharp] + severity: ERROR + message: >- + Newtonsoft TypeNameHandling other than None lets the payload choose the + CLR type to instantiate, which is a remote-code-execution gadget chain. + Use TypeNameHandling.None (the default) or a SerializationBinder. + metadata: + category: security + cwe: "CWE-502" + patterns: + - pattern-either: + - pattern: TypeNameHandling.All + - pattern: TypeNameHandling.Auto + - pattern: TypeNameHandling.Objects + - pattern: TypeNameHandling.Arrays + + - id: csharp-cert-validation-always-true + languages: [csharp] + severity: ERROR + message: >- + The TLS certificate validation callback unconditionally returns true, so + every certificate is accepted (MITM). Remove the override. + metadata: + category: security + cwe: "CWE-295: Improper Certificate Validation" + patterns: + - pattern-either: + - pattern: $X.ServerCertificateValidationCallback = (...) => true; + - pattern: $X.ServerCertificateCustomValidationCallback = (...) => true; + - pattern: ServicePointManager.ServerCertificateValidationCallback = (...) => true; + - pattern: $X.RemoteCertificateValidationCallback = (...) => true; + + - id: csharp-weak-hash-md5-sha1 + languages: [csharp] + severity: ERROR + message: >- + Weak hash algorithm (MD5/SHA-1). For any security purpose use SHA-256+. + For non-security content hashing prefer a non-crypto hash so intent is + explicit. + metadata: + category: security + cwe: "CWE-327: Use of a Broken or Risky Cryptographic Algorithm" + patterns: + - pattern-either: + - pattern: MD5.Create(...) + - pattern: SHA1.Create(...) + - pattern: new MD5CryptoServiceProvider(...) + - pattern: new SHA1CryptoServiceProvider(...) + - pattern: new MD5Managed(...) + - pattern: new SHA1Managed(...) + - pattern: HashAlgorithm.Create("MD5") + - pattern: HashAlgorithm.Create("SHA1") + + - id: csharp-process-start-dynamic-command + languages: [csharp] + severity: ERROR + message: >- + Process.Start called with a command string built at runtime + (interpolation / concatenation / string.Format). Pass a ProcessStartInfo + with a literal FileName and an explicit ArgumentList instead. + metadata: + category: security + cwe: "CWE-78: OS Command Injection" + # Written as a flat pattern-either on purpose. The equivalent + # `pattern: Process.Start($CMD, ...)` + `metavariable-pattern` form TIMED OUT + # (opengrep default 5s/rule/file) on src/SwfocTrainer.Runtime/Services/ + # RuntimeAdapter.cs (6187 lines / 230 KB), which opengrep reports as a + # *warning* while still exiting 0 -- i.e. the rule silently stopped covering + # that file. Keep this shape cheap. + patterns: + - pattern-either: + - pattern: Process.Start($A + $B, ...) + - pattern: Process.Start(string.Format(...), ...) + - pattern: Process.Start(String.Format(...), ...) + - pattern: Process.Start($"...", ...) + + - id: csharp-assembly-load-dynamic + languages: [csharp] + severity: ERROR + message: >- + Assembly loaded from a runtime-computed path/byte array. Anything that can + write that path gains code execution in this process. Load only from a + fixed, verified location. + metadata: + category: security + cwe: "CWE-829: Inclusion of Functionality from Untrusted Control Sphere" + patterns: + - pattern-either: + - pattern: Assembly.Load($X) + - pattern: Assembly.LoadFrom($X) + - pattern: Assembly.LoadFile($X) + - pattern: Assembly.UnsafeLoadFrom($X) + - pattern-not: Assembly.Load("...") + - pattern-not: Assembly.LoadFrom("...") + - pattern-not: Assembly.LoadFile("...") + - pattern-not: Assembly.UnsafeLoadFrom("...") diff --git a/.quality/opengrep/general-security.yaml b/.quality/opengrep/general-security.yaml new file mode 100644 index 000000000..0911527bf --- /dev/null +++ b/.quality/opengrep/general-security.yaml @@ -0,0 +1,42 @@ +# Curated language-agnostic security rules (subset of p/r2c-security-audit). +# opengrep/semgrep compatible. See README.md. +rules: + - id: generic-private-key-committed + languages: [generic] + severity: ERROR + message: >- + A PEM private key block appears to be committed to source. Private keys + must never live in the repo; use a secret manager / env vars. + metadata: + category: security + cwe: "CWE-798: Use of Hard-coded Credentials" + paths: + exclude: + - "*.example" + - ".env.example" + - "**/tests/**" + - "**/fixtures/**" + - ".quality/opengrep/**" + - ".gitleaks.toml" + patterns: + - pattern-regex: "-----BEGIN (RSA |EC |OPENSSH |DSA |PGP )?PRIVATE KEY-----" + + - id: generic-aws-access-key-id + languages: [generic] + severity: ERROR + message: >- + A string matching an AWS Access Key ID was found. Rotate it and move + credentials to a secret manager / environment variables. + metadata: + category: security + cwe: "CWE-798" + paths: + exclude: + - "*.example" + - ".env.example" + - "**/tests/**" + - "**/fixtures/**" + - ".quality/opengrep/**" + - ".gitleaks.toml" + patterns: + - pattern-regex: "\\b(AKIA|ASIA)[0-9A-Z]{16}\\b" diff --git a/.quality/opengrep/python-security.yaml b/.quality/opengrep/python-security.yaml new file mode 100644 index 000000000..817928445 --- /dev/null +++ b/.quality/opengrep/python-security.yaml @@ -0,0 +1,119 @@ +# Curated Python security rules for the lean gate-4 (opengrep / Semgrep CE). +# Covers ALL Python in this repo (scripts/, tools/, bridge/, swfoc_toolkit/, +# swfoc_lua_bridge/, tests/python/) -- deliberately WIDER than ruff.toml's +# extend-exclude, because a security lane should not inherit a style lane's +# scope. See README.md. +rules: + - id: python-exec-use + languages: [python] + severity: ERROR + message: >- + Use of exec() detected. Executing dynamically-built code is dangerous and + can lead to arbitrary code execution. Use explicit dispatch. + metadata: + category: security + cwe: "CWE-95: Improper Neutralization of Directives in Dynamically Evaluated Code" + patterns: + - pattern: exec(...) + + - id: python-eval-use + languages: [python] + severity: ERROR + message: >- + Use of eval() detected. eval on untrusted input enables code injection. + Use ast.literal_eval or an explicit parser instead. + metadata: + category: security + cwe: "CWE-95" + patterns: + - pattern: eval(...) + + - id: python-subprocess-shell-true + languages: [python] + severity: ERROR + message: >- + subprocess called with shell=True. If any argument is attacker-influenced + this allows command injection. Pass an argument list and shell=False. + metadata: + category: security + cwe: "CWE-78: OS Command Injection" + patterns: + - pattern: subprocess.$F(..., shell=True, ...) + + - id: python-os-system + languages: [python] + severity: ERROR + message: >- + os.system()/os.popen() invokes a shell and is prone to command injection. + Use subprocess with an argument list instead. + metadata: + category: security + cwe: "CWE-78" + patterns: + - pattern-either: + - pattern: os.system(...) + - pattern: os.popen(...) + + - id: python-yaml-unsafe-load + languages: [python] + severity: ERROR + message: >- + yaml.load without SafeLoader (or yaml.unsafe_load) can construct arbitrary + Python objects. Use yaml.safe_load(). + metadata: + category: security + cwe: "CWE-502: Deserialization of Untrusted Data" + patterns: + - pattern-either: + - pattern: yaml.load($X) + - pattern: yaml.unsafe_load(...) + - pattern: yaml.load(..., Loader=yaml.Loader, ...) + - pattern: yaml.load(..., Loader=yaml.UnsafeLoader, ...) + + - id: python-pickle-loads + languages: [python] + severity: ERROR + message: >- + pickle/marshal deserialization of untrusted data enables arbitrary code + execution. Use a safe format (JSON) for anything crossing a trust boundary. + metadata: + category: security + cwe: "CWE-502" + patterns: + - pattern-either: + - pattern: pickle.loads(...) + - pattern: pickle.load(...) + - pattern: cPickle.loads(...) + - pattern: marshal.loads(...) + + - id: python-weak-hash-md5-sha1 + languages: [python] + severity: ERROR + message: >- + Weak hash algorithm (md5/sha1) used. For security contexts use sha256+. + If this is non-security content hashing, pass usedforsecurity=False to + make the intent explicit. + metadata: + category: security + cwe: "CWE-327: Use of a Broken or Risky Cryptographic Algorithm" + patterns: + - pattern-either: + - pattern: hashlib.md5($X) + - pattern: hashlib.sha1($X) + - pattern: hashlib.new("md5", ...) + - pattern: hashlib.new("sha1", ...) + + - id: python-tls-verification-disabled + languages: [python] + severity: ERROR + message: >- + TLS certificate verification disabled. This permits man-in-the-middle + attacks. Keep verification enabled. + metadata: + category: security + cwe: "CWE-295: Improper Certificate Validation" + patterns: + - pattern-either: + - pattern: requests.$F(..., verify=False, ...) + - pattern: ssl._create_unverified_context(...) + - pattern: ssl._create_default_https_context = ssl._create_unverified_context diff --git a/swfoc_toolkit/live_test.py b/swfoc_toolkit/live_test.py index ba8bea535..be519ad4f 100644 --- a/swfoc_toolkit/live_test.py +++ b/swfoc_toolkit/live_test.py @@ -106,6 +106,13 @@ def deploy_dll() -> bool: def launch_game() -> bool: """Launch the game via Steam.""" print(f"[*] Launching game via Steam (AppID {STEAM_APP_ID})...") + # The argv is a fixed literal list whose only interpolation is the + # module-level STEAM_APP_ID constant, so no untrusted input can reach the + # shell. shell=True is also redundant here (the list already invokes + # `cmd /c`) and should be dropped -- tracked as a follow-up rather than + # changed inside a ruleset-adoption PR, because this harness drives a real + # game process and cannot be exercised in CI. + # nosemgrep: python-subprocess-shell-true -- literal argv, constant AppID subprocess.Popen( ["cmd", "/c", f"start steam://rungameid/{STEAM_APP_ID}"], shell=True