Skip to content

security: systematic XSS remediation across all templates#97

Open
rhoerr wants to merge 4 commits intovpietri:mainfrom
rhoerr:qdb/pr4-xss-sweep
Open

security: systematic XSS remediation across all templates#97
rhoerr wants to merge 4 commits intovpietri:mainfrom
rhoerr:qdb/pr4-xss-sweep

Conversation

@rhoerr
Copy link
Copy Markdown

@rhoerr rhoerr commented Apr 5, 2026

Summary

  • Escape all variable output in templates with escapeHtml/escapeHtmlAttr/escapeUrl
  • Rewrite formatSql() to escape input before applying syntax highlighting
  • Escape IDE link helper HTML output with htmlspecialchars
  • Replace innerHTML with textContent in log viewer
  • Remove executeScriptElements() which re-activated scripts from AJAX HTML

Security Findings Addressed

ID Finding Severity
H6 formatSql() applies regex to raw SQL without escaping High
H7 Templates output variables without escapeHtml High
H8 executeScriptElements re-runs scripts from innerHTML High
M5 IDE link helper outputs unescaped file paths Medium

Files Modified

  • Block/Tab/Content/Sql.php — formatSql() escape-first
  • Block/Tab/Content/Request.php — formatValue() escaping
  • Helper/Data.php — getIDELinkForFile() htmlspecialchars
  • 9 template files — escapeHtml/escapeHtmlAttr/escapeUrl applied
  • toolbar.phtml — executeScriptElements removed
  • log.phtml — innerHTML → textContent

Test plan

  • Verify SQL tab still highlights keywords with colored spans
  • Verify event/observer/cache tabs render correctly with escaped output
  • Verify log viewer displays log content as plain text
  • Verify IDE links still work when IDE integration is configured
  • Verify no raw HTML injection possible via crafted SQL or event names

🤖 Generated with Claude Code

rhoerr and others added 4 commits April 5, 2026 18:21
- Escape all variable output in templates with escapeHtml/escapeHtmlAttr/escapeUrl
- Rewrite formatSql() to escape input before applying syntax highlighting
- Escape IDE link helper HTML output with htmlspecialchars
- Replace innerHTML with textContent in log viewer
- Remove executeScriptElements() (re-activates scripts from AJAX HTML)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous commit removed executeScriptElements and its call as a CSP
hardening measure. However, four tab templates (log, action, profiler,
dumper) contain inline <script> blocks that are loaded via AJAX into
innerHTML. Browsers do not execute scripts set via innerHTML, so
executeScriptElements (which clones script elements to force execution)
is required for those tabs to function.

Removing executeScriptElements is a valid goal but requires refactoring
all tab templates to use event-delegation or post-load initialization
instead of inline scripts — out of scope for this XSS sweep.

The stray this.log() debug call has been removed from the restored method.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rhoerr rhoerr marked this pull request as ready for review April 6, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant