Skip to content

Chat formatter: audit pre-existing formatToolResult cases for prose-interpolation XSS #32

@mwtcmi

Description

@mwtcmi

Background

GHSA-7qvv (v1.6.6) patched stored XSS in the chat formatter for content inside its markdown capture groups (backticks, bold, links, {{cmd:}}, {{download:}}). Prose text rendered between those patterns is still inserted via jQuery .append() with innerHTML semantics — no escaping.

v1.7.1 introduced Frogman.class.php::sanitizeForChat() and applied it to the five new fm_audit_* formatter cases. Every user-controlled field there is now sanitizeForChat()-ed and wrapped in backticks. But the helper hasn't been retrofitted to pre-existing formatter cases, and some of them interpolate user-controlled fields directly into prose.

Scope

Walk every case in Frogman.class.php::formatToolResult(). For each, classify each interpolated field as:

  • Frogman-controlled (tool names, severity labels, fixed-string issues / recommendations / icons / drilldown phrases) — safe to interpolate raw
  • User-controlled (description, name, comment, free-form parameter, anything an admin can type into a FreePBX field that we then read back) — must be sanitized and backtick-wrapped

For each user-controlled field that's currently raw, apply the v1.7.1 pattern:

$sanitized = $this->sanitizeForChat($f['fieldname']);
$line[] = "Field `{$sanitized}`";

Known suspect

  • Frogman.class.php fm_dialplan_show case — interpolates $ctx['comment'] directly into prose. Found during the v1.7.1 work, not yet fixed.

There are almost certainly more. The list is bounded — one walk of formatToolResult will find every case.

Severity framing

Same shape as GHSA-7qvv (multi-admin / tier-confused-deputy stored XSS). Each individual finding may be LOW on its own depending on the data source, but the cumulative surface — every formatter case where an admin can put a payload into a FreePBX object that another admin renders — adds up. If any case turns out to be reflectable by a non-admin user, that one becomes GHSA-class on its own.

Recommendation: do the walk, fix everything found in a single sweep, file a GHSA if any reflectable-by-non-admin path turns up. If not, ship as a hardening release with a security-conscious note.

How to apply

  1. grep -n "case '" Frogman.class.php to enumerate every formatToolResult case
  2. For each, read the case body and identify every {$...} interpolation
  3. Trace each interpolation back to its source — is the field set by an admin in FreePBX, by Frogman itself, by Asterisk, by a non-admin user (extension owners via UCP)?
  4. For every admin-or-lower-set field, apply the v1.7.1 pattern
  5. Re-run the v1.7.1 injection-test fixture (six vectors: <script>, backtick breakout, CRLF/NUL, {{cmd:}}, [link](url), **bold**) against the patched cases
  6. If any path turns out to be reflectable by non-admin users, file a GHSA before merging the public fix

References

  • v1.7.1 release notes — section "Security hardening" — describes sanitizeForChat and the injection vectors
  • GHSA-7qvv-... — original chat-formatter XSS (v1.6.6) — same risk class, narrower scope
  • feedback_chat_formatter_xss_pattern.md (Claude memory, internal) — the rule and the rationale

Discovered

While building v1.7.1's audit-finding chat rendering on 2026-05-18. Mike caught the gap pre-ship and asked for the broader sweep to be filed as a follow-up.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions