fix: html-escape captured values in helpcenter article markdown embeds (#14140)
Embed templates interpolate regex captures from user-authored article URLs into HTML attribute values. CommonMark's angle-bracket link destination syntax allows characters that the capture regexes don't filter, so the unescaped substitution could produce malformed attribute output. Escaping at substitution time keeps the render deterministic regardless of the URL. ### How was this tested? Added specs. Fixes [CW-6934](https://linear.app/chatwoot/issue/CW-6934/) Co-authored-by: Sony Mathew <sony@chatwoot.com> Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
This commit is contained in:
co-authored by
Sony Mathew
Sivin Varghese
parent
941c8a86b4
commit
2192af80f4
@@ -77,9 +77,10 @@ class CustomMarkdownRenderer < CommonMarker::HtmlRenderer
|
||||
return nil unless embed_config
|
||||
|
||||
template = embed_config['template']
|
||||
# Use Ruby's built-in named captures with gsub to handle CSS % values
|
||||
# Use gsub (not format) so CSS `%` values in templates don't need escaping.
|
||||
# Captured values are HTML-escaped since they land inside HTML attribute contexts.
|
||||
match_data.named_captures.each do |var_name, value|
|
||||
template = template.gsub("%{#{var_name}}", value)
|
||||
template = template.gsub("%{#{var_name}}", CGI.escapeHTML(value))
|
||||
end
|
||||
template
|
||||
end
|
||||
|
||||
@@ -238,5 +238,23 @@ describe CustomMarkdownRenderer do
|
||||
expect(output).to include('allow="accelerometer; gyroscope; autoplay; encrypted-media; picture-in-picture;"')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when captured values contain HTML-special characters' do
|
||||
# CommonMark angle-bracket link destinations `[text](<URL>)` permit characters
|
||||
# like `"` that the embed regex captures would otherwise pass through raw into
|
||||
# attribute values. Captures are HTML-escaped before interpolation so the
|
||||
# substituted value cannot break out of the surrounding attribute context.
|
||||
it 'escapes double quotes in captured YouTube video_id' do
|
||||
markdown = "\n[demo](<https://www.youtube.com/watch?v=x\" onload=\"alert(1)>)\n"
|
||||
output = render_markdown(markdown)
|
||||
expect(output).not_to include('onload="alert(1)"')
|
||||
expect(output).to include('"')
|
||||
end
|
||||
|
||||
it 'leaves legitimate alphanumeric IDs untouched' do
|
||||
output = render_markdown_link('https://www.youtube.com/watch?v=dQw4w9WgXcQ')
|
||||
expect(output).to include('src="https://www.youtube-nocookie.com/embed/dQw4w9WgXcQ"')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user