security: OWASP audit and hardening (v1.0.8)
All checks were successful
Create Release Package / PHP Lint (push) Successful in 1m8s
Create Release Package / Build Release (push) Successful in 1m53s

- Archive XSS: wrap get_the_archive_title/description with wp_kses_post()
  in ContextBuilder to sanitize Editor-editable term content rendered via |raw
- Comment fields: esc_html() on comment_author, esc_url() on comment_author_url
  at data source; template updated to output pre-escaped URL via |raw
- dark-mode.js: whitelist localStorage value against ['dark','light'] to
  prevent attribute injection from third-party script tampering
- TwigService: add is_safe=>html to esc_html/esc_attr/esc_url Twig functions
  to prevent double-encoding if autoescape is ever enabled
- Add .markdownlint.json (disable MD024 duplicate headings, MD013 line length)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-19 13:23:33 +01:00
parent 876be4a041
commit 89afa00678
9 changed files with 78 additions and 12 deletions

View File

@@ -211,6 +211,44 @@ Build steps (in order):
## Session History
### Session 14 — v1.0.8 Security Audit & Hardening (2026-02-19)
**Completed:** Comprehensive OWASP-aligned security audit. Two parallel background agents reviewed all PHP (functions.php, ContextBuilder, NavWalker, TemplateController, TwigService, all patterns) and JavaScript/Twig templates. Four targeted security fixes applied.
**Findings and fixes:**
- **Archive term description XSS (High)**: `get_the_archive_description()` returns raw term content editable by Editor-role users. Templates rendered it with `|raw`, creating a stored XSS path. Fixed: wrapped with `wp_kses_post()` in `ContextBuilder::getArchiveData()`. Same applied to `get_the_archive_title()`.
- **Comment author injection (Low, defense-in-depth)**: `comment_author` and `comment_author_url` were passed to Twig as raw database values. Fixed: `esc_html()` applied to author name, `esc_url()` applied to author URL in `ContextBuilder::buildCommentTree()`. Template updated to output pre-escaped URL via `|raw` rather than calling `esc_url()` in Twig.
- **Dark mode localStorage whitelist (Medium)**: `getPreferredTheme()` returned any stored value without validation, allowing attribute injection if a malicious script wrote to `localStorage`. Fixed: strict equality check against `['dark', 'light']` before trusting the stored value.
- **Twig escaping functions not marked safe (Medium)**: `esc_html()`, `esc_attr()`, `esc_url()` registered in `TwigService` lacked `['is_safe' => ['html']]`, meaning any future autoescape enablement would cause double-encoding. Fixed: all three now carry the `is_safe` declaration.
**Confirmed secure (no action needed):**
- All `|raw` filter usages for widget HTML, comment content, comment reply links, and comment forms are by-design (WordPress core output)
- Pattern files: no direct-access guards needed (loaded only via `register_block_pattern()`)
- No SQL injection vectors (`$wpdb` not used directly; all data via WordPress functions)
- `TemplateController` error handling: `\Throwable` caught, logged, and gated behind `WP_DEBUG`
- `do_shortcode()` and `wp_kses_post()` Twig functions correctly marked `is_safe`
- `wp_head()`, `wp_footer()`, `body_class()` Twig functions correctly use output buffering + `is_safe`
**Key learnings:**
- WordPress Twig themes should not enable `autoescape => 'html'` globally: `get_the_title()` applies `wptexturize()` which returns HTML entities (`&mdash;`, `&ldquo;`). Autoescape would double-encode these, corrupting post title rendering.
- `esc_url()` does more than HTML-encoding — it validates the URL scheme and strips dangerous protocols (`javascript:`, `data:`). Always use it for user-supplied URLs, even when autoescape is active.
- Registering WordPress escaping functions (`esc_url`, `esc_html`, `esc_attr`) as Twig functions without `is_safe => html` silently creates a double-encoding trap: calling `{{ esc_url(url) }}` with autoescape on would produce `&amp;amp;` instead of `&amp;`.
- Added `.markdownlint.json` disabling MD024 (duplicate headings, expected in changelogs) and MD013 (line length).
**Files modified:**
- `inc/Template/ContextBuilder.php` — archive data sanitization, comment field escaping
- `inc/Twig/TwigService.php``is_safe => html` on three escaping functions
- `views/partials/comment-item.html.twig` — use pre-escaped author URL
- `src/js/dark-mode.js` — localStorage whitelist
- `assets/js/dark-mode.js` — rebuilt compiled output
- `style.css` — version bump to 1.0.8
- `CHANGELOG.md` — v1.0.8 entry
- `.markdownlint.json` — created
### Session 13 — v1.0.5 Translation Files (2026-02-11)
**Completed:** Standardized translation file naming and added 11 new locale translations.