Pārlūkot izejas kodu

docs: mark SEC_REVIEW F52 as fixed in e5b525b

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa 3 dienas atpakaļ
vecāks
revīzija
6f5429e958
1 mainītis faili ar 28 papildinājumiem un 1 dzēšanām
  1. 28 1
      doc/SEC_REVIEW.md

+ 28 - 1
doc/SEC_REVIEW.md

@@ -11,7 +11,7 @@
 >
 > Each finding is referenced as **F<N>** for later citation.
 >
-> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (19 fixed, 23 open).
+> **Findings rolled up:** 5 sev-3 (5 fixed, 0 open), 27 sev-2 (27 fixed, 0 open), 42 sev-1 (20 fixed, 22 open).
 
 ---
 
@@ -1711,6 +1711,33 @@
   downstream. Strip control chars and NFC-normalise at the input
   boundary.
 - **Severity: 1**
+- **Status:** Fixed. New `AdminControllerSupport::stripControlChars`
+  and `cleanString` helpers strip C0 (`0x00..0x1f`), DEL (`0x7f`),
+  and C1 (`0x80..0x9f`) bytes. Applied at every relevant call site
+  on both create and update paths in:
+  - `ReportersController` — `name`, `description`,
+  - `ConsumersController` — `name`, `description`,
+  - `CategoriesController` — `name`, `description` (slug is already
+    regex-validated to lowercase + `_`-only),
+  - `ManualBlocksController` — `reason`,
+  - `AllowlistController` — `reason`.
+  The strip removes the ESC byte that leads ANSI escape sequences,
+  so terminal-interpretation attacks on log viewers (`\u{001b}[31m`)
+  collapse — the trailing `[31m` text remains visible but inert
+  without the lead-in. NULs and newlines (`\n[CRIT] fake event`)
+  are gone outright. NFC normalisation was deliberately skipped —
+  the api doesn't require `ext-intl` and adding the dependency for
+  a defence-in-depth nice-to-have isn't worth the install-footprint
+  cost. The `details_json` audit blob inherits the scrub because
+  the controllers feed the cleaned `name` / `description` / `reason`
+  into the audit emit. Regression tests:
+  `api/tests/Integration/Admin/InputControlCharStrippingTest.php` —
+  one POST per controller plus a PATCH on the reporter update path,
+  asserting both that the control bytes are gone (`preg_match(
+  '/[\x00-\x1f\x7f-\x9f]/u', value) === 0`) and that the
+  surrounding visible payload round-trips byte-for-byte. The
+  reporter case also drills into `audit_log.target_label` and
+  `details_json` to prove the audit row never sees the raw bytes.
 
 ### F53 — Stored-XSS sink potential in `categories/edit.twig` `decay_function`
 - **File:** `ui/resources/views/pages/categories/edit.twig:7-10`