Skip to content

[P0] Security: Fix XSS in property-description-textarea link sanitization #23

Description

@Pamacea

[P0] Security: Fix XSS in property-description-textarea link sanitization

Severity

  • Priority: P0
  • Type: Security (XSS)
  • Estimation: 3 hours

Problem

The PropertyDescriptionTextarea component processes markdown links BEFORE final sanitization, creating a window for XSS attacks. The link replacement regex on lines 84-92 creates raw HTML with single quotes that may not be properly sanitized.

Vulnerable Code

// src/shared/ui/properties/property-description-textarea.tsx:84-92
processed = processed.replace(/\[(.+?)\]\((.+?)\)/g, (match, text, url) => {
  const sanitizedUrl = url.trim().toLowerCase();
  if (sanitizedUrl.startsWith("http://") || sanitizedUrl.startsWith("https://")) {
    // DANGER: Creating HTML with single quotes before final sanitization
    return `<a href='${url}' class='text-accent-gold hover:underline' target='_blank' rel='noopener noreferrer'>${text}</a>`;
  }
  return text;
});
// Then line 94: dangerouslySetInnerHTML={{ __html: sanitizeHtml(processed) }}

Attack Vector

An attacker could inject:

[click](https://example.com' onmouseover='alert(document.cookie)')

The link replacement would produce:

<a href='https://example.com' onmouseover='alert(document.cookie)''

While DOMPurify should catch this, the current sanitizeHtml allows class and data-* attributes which could be exploited if DOMPurify isn't properly configured.

Impact

  • XSS attack allowing script execution in user context
  • Potential session hijacking
  • Data theft via malicious links in descriptions

Files Affected

  • src/shared/ui/properties/property-description-textarea.tsx:84-94
  • src/shared/lib/sanitize.ts - Need to verify DOMPurify configuration

Steps to Fix

  1. Install DOMPurify properly (not via dynamic require):
npm install dompurify @types/dompurify
  1. Fix the link sanitization:
processed = processed.replace(/\[(.+?)\]\((.+?)\)/g, (match, text, url) => {
  // Use double quotes and sanitize URL properly
  const sanitizedUrl = sanitizeUrl(url);
  if (!sanitizedUrl) return text; // Don't render unsafe URLs
  
  const safeText = escapeHtml(text);
  return `<a href="${sanitizedUrl}" class="text-accent-gold hover:underline" target="_blank" rel="noopener noreferrer">${safeText}</a>`;
});
  1. Update sanitize.ts:
import DOMPurify from 'dompurify';

export function sanitizeHtml(html: string, options?: any): string {
  if (!html) return "";
  
  return DOMPurify.sanitize(html, {
    ALLOWED_TAGS: ["p", "br", "strong", "b", "em", "i", "a", "h1", "h2", "h3", "h4", "h5", "h6", "ul", "ol", "li", "blockquote", "code", "pre", "span"],
    ALLOWED_ATTR: ["href", "class", "rel", "target"],
    // Force rel="noopener" on all links
    FORBID_ATTR: ["onclick", "onload", "onerror"],
    // Don't allow data: or javascript: URLs
    FORBID_TAGS: ["script", "object", "embed", "iframe"],
  });
}
  1. Add XSS tests:
describe('PropertyDescriptionTextarea', () => {
  it('should sanitize XSS in markdown links', () => {
    const malicious = "[click](javascript:alert(1))";
    const { container } = render(<Component value={malicious} />);
    expect(container.innerHTML).not.toContain('javascript:');
  });
  
  it('should sanitize onmouseover in links', () => {
    const malicious = "[click](https://safe.com' onmouseover='alert(1)')";
    const { container } = render(<Component value={malicious} />);
    expect(container.innerHTML).not.toContain('onmouseover');
  });
});

Acceptance Criteria

  • DOMPurify properly installed and imported
  • Link URLs sanitized before HTML creation
  • Link text escaped with escapeHtml()
  • Unit tests for XSS prevention pass
  • Integration test verifies no script execution

Notes

Similar issue may exist in lore-detail-client and world-client if they use similar markdown processing.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    criticalCritical issue requiring immediate attentionp0Priority: CriticalsecuritySecurity vulnerability or fix

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions