GHSA-X975-RGX4-5FH4: Root-cause XSS Fix in appium-mcp MCP-UI

Vulnerability

GHSA-X975-RGX4-5FH4 describes a cross-site scripting issue in the Appium MCP server's MCP-UI resource. The vulnerable UI builder interpolated untrusted layout metadata directly into HTML and into an inline onclick handler. Per the commit diff in the patch source, attacker-controlled values such as tagName, text, contentDesc, resourceId, locator strategy, and selector were inserted without HTML escaping.

This created multiple execution paths: HTML body injection via element content, attribute injection via interpolated values, and JavaScript-context injection via the inline handler onclick="testLocator('...')". Given the advisory's impact statement, successful script execution in the inspector WebView could be chained into host command execution through message-passing abuse.

<h3>${element.tagName}</h3>
${element.text ? `<p class="element-text"><strong>Text:</strong> ${element.text}</p>` : ''}
${element.contentDesc ? `<p class="element-text"><strong>Content Desc:</strong> ${element.contentDesc}</p>` : ''}
${element.resourceId ? `<p class="element-text"><strong>Resource ID:</strong> <code>${element.resourceId}</code></p>` : ''}
<button class="test-btn" onclick="testLocator('${strategy}', \`${selector.replace(/`/g, '\\`')}\`)">Test</button>

The vulnerable pattern is source-grounded in the commit diff at GitHub commit e222bbbd6fe2b656a320efcd143563f08061a83d. Additional context is summarized by the advisory and report pages: GitHub Advisory, CVE Reports entry.

Patch

The patch makes two substantive changes. First, it introduces a centralized escapeHtml function and applies it to every untrusted value rendered into the UI. Second, it removes the inline onclick sink entirely and replaces it with delegated event handling that reads escaped values from data-* attributes.

<h3>${escapeHtml(element.tagName)}</h3>
${element.text ? `<p class="element-text"><strong>Text:</strong> ${escapeHtml(element.text)}</p>` : ''}
${element.contentDesc ? `<p class="element-text"><strong>Content Desc:</strong> ${escapeHtml(element.contentDesc)}</p>` : ''}
${element.resourceId ? `<p class="element-text"><strong>Resource ID:</strong> <code>${escapeHtml(element.resourceId)}</code></p>` : ''}
<span class="strategy">${escapeHtml(strategy)}</span>
<code class="selector">${escapeHtml(selector)}</code>
<button class="test-btn" data-strategy="${escapeHtml(strategy)}" data-selector="${escapeHtml(selector)}">Test</button>

document.addEventListener('click', (event) => {
  const target = event.target;
  const button = target instanceof Element ? target.closest('.test-btn') : null;
  if (!button) {
    return;
  }

  testLocator(button.dataset.strategy || '', button.dataset.selector || '');
});

function escapeHtml(value: unknown): string {
  return String(value)
    .replace(/&/g, '&amp;')
    .replace(/</g, '&lt;')
    .replace(/>/g, '&gt;')
    .replace(/"/g, '&quot;')
    .replace(/'/g, '&#039;');
}

The accompanying test is strong evidence of intent and coverage. It feeds malicious values into all relevant fields and asserts that raw tags, event handlers, and injected HTML are absent from the generated output while escaped equivalents remain present. This directly validates both the content escaping and the removal of inline handler injection, as shown in the patch commit.

Review

Pros

• Addresses the actual sink classes visible in the diff: HTML text-node injection and inline event-handler injection.
• Uses a single escaping primitive consistently across all user-controlled fields rendered by the UI generator.
• Eliminates inline JavaScript construction, which is materially safer than trying to quote-escape attacker input inside an onclick attribute.
• Switches to event delegation with data-strategy and data-selector, reducing exposure to script-context parsing bugs.
• Adds a regression test with realistic payloads including tag injection, attribute injection, and postMessage-oriented payload content.

Cons

• The reviewable patch scope is limited to src/ui/mcp-ui-utils.ts; it does not prove that all other UI rendering paths in the project follow the same escaping discipline.
• escapeHtml is context-specific. It is correct for HTML text and double-quoted attribute contexts used here, but it would not be sufficient for URL, CSS, or script contexts if future code reuses it incorrectly.
• The delegated click handler trusts dataset values as plain strings. That is acceptable for this sink, but downstream consumers such as testLocator still need their own validation for protocol or command boundaries.

Verdict

Root-cause.

The patch fixes the root cause exposed by the cited sources: untrusted locator metadata was being inserted into executable and parseable HTML/JS contexts without output encoding. By escaping all rendered fields and removing the inline onclick sink, the patch closes the demonstrated XSS vectors rather than merely filtering a subset of payloads. Based on the available diff and tests in the commit, this is a technically sound remediation for the vulnerable MCP-UI generator described in the advisory.