Security: Prevent HTML/CSS injection in markdown rendering#87
Conversation
- Keep forced styling on images with max-height constraint - Remove dangerous HTML tags (style, link, meta) from markdown output - Improve security sanitization for markdown rendering
- Force all img tags to have style='max-height: 150px;' regardless of source - Override any user-provided styles on images - Maintain consistent image sizing across markdown and raw HTML
- Prevent raw HTML injection by removing all HTML tags before parsing - Block dangerous HTML like <div style="..."> from being processed - Ensure only markdown-generated HTML can be rendered
|
Caution Review failedThe pull request is closed. WalkthroughThe PR implements a security-focused sanitization pipeline for markdown rendering in ircUtils.tsx. Input text is stripped of HTML tags, markdown is parsed, then output is filtered to allow only whitelisted HTML elements. Dangerous content (scripts, iframes, styles, links) is removed, event handlers are stripped, image styles are normalized, and input validation with length caps is added to prevent XSS and DoS attacks. Changes
Sequence DiagramsequenceDiagram
participant User
participant renderMarkdown
participant Sanitization
participant MarkdownParser
participant Validator
User->>renderMarkdown: Input markdown text
rect rgb(230, 240, 250)
Note over renderMarkdown: New Security Pipeline
renderMarkdown->>Validator: Check length & validity
Validator-->>renderMarkdown: Valid/Invalid
end
alt Invalid Input
renderMarkdown-->>User: Return text as-is
else Valid Input
renderMarkdown->>Sanitization: Strip HTML tags
Sanitization->>MarkdownParser: Clean input
MarkdownParser->>Sanitization: Parse to HTML
rect rgb(240, 230, 250)
Note over Sanitization: Filter & Normalize
Sanitization->>Sanitization: Apply allowlist
Sanitization->>Sanitization: Remove dangerous tags
Sanitization->>Sanitization: Strip event handlers
Sanitization->>Sanitization: Normalize img styles
end
Sanitization-->>renderMarkdown: Sanitized HTML
renderMarkdown-->>User: Safe rendered content
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Reasoning: The change introduces security-critical sanitization logic with moderate complexity. While focused to a single file, the logic involves multiple interconnected security concerns (whitelist validation, dangerous content removal, event handler stripping, input validation), each requiring careful scrutiny. The implementation pattern is consistent but security-sensitive, necessitating thorough review of edge cases and bypass prevention. Possibly Related PRs
Suggested Reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
) * Remove style attributes from markdown-rendered HTML and enhance security - Keep forced styling on images with max-height constraint - Remove dangerous HTML tags (style, link, meta) from markdown output - Improve security sanitization for markdown rendering * Ensure all img tags have controlled max-height styling - Force all img tags to have style='max-height: 150px;' regardless of source - Override any user-provided styles on images - Maintain consistent image sizing across markdown and raw HTML * lint * Strip all HTML tags from input before markdown processing - Prevent raw HTML injection by removing all HTML tags before parsing - Block dangerous HTML like <div style="..."> from being processed - Ensure only markdown-generated HTML can be rendered
This PR implements comprehensive security fixes for markdown rendering to prevent HTML and CSS injection attacks.
Security Fixes
1. Raw HTML Prevention
<div style="background:black">2. HTML Tag Whitelisting
div,span, etc.3. Style Attribute Sanitization
styleattributes from HTML tagsmax-height: 150px)4. Enhanced Sanitization
Impact
Testing
Summary by CodeRabbit