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