Skip to content

Commit 5e0ea99

Browse files
kouclaude
andauthored
Don't link URLs with unsafe schemes in Markup::ToHtml (#1832)
`{click}[javascript:alert(1)]` and `[click](javascript:alert(1))` were rendered as `<a href="javascript:alert(1)">`. The test added for this in #933 checked the input string instead of the converted result, so it always passed. `gen_url` now renders only the link text when the resolved URL has a `javascript:`, `vbscript:`, `data:` or `file:` scheme, the same schemes cmark and markdown-it reject: * https://github.com/commonmark/cmark/blob/6566d91bdb25898bd910f8bed2a3f18f4a05cf91/src/scanners.re#L322-L332 * https://github.com/markdown-it/markdown-it/blob/3c51991c32aaa2b002a52c009334ebe5752c84b3/src/markdownit.ts#L60-L61 The check is done after `parse_url` so that `link:javascript:...` is also caught, and ignores leading C0 controls and spaces and ASCII tabs and newlines like browsers do: * https://url.spec.whatwg.org/#concept-basic-url-parser Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 4355a42 commit 5e0ea99

2 files changed

Lines changed: 52 additions & 1 deletion

File tree

‎lib/rdoc/markup/to_html.rb‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ def initialize(pipe: false, output_decoration: true)
148148
#
149149
# These methods are used by regexp handling markup added by RDoc::Markup#add_regexp_handling.
150150

151+
##
152+
# URL schemes that are not linked because they can run scripts or open
153+
# local files. Links to them are rendered as their text only.
154+
155+
UNSAFE_URL_SCHEME_REGEXP = /\A(?:javascript|vbscript|data|file):/i
156+
151157
# :nodoc:
152158
URL_CHARACTERS_REGEXP_STR = /[A-Za-z0-9\-._~:\/\?#\[\]@!$&'\(\)*+,;%=]/.source
153159

@@ -605,10 +611,15 @@ def convert_string(text)
605611
# - File references (.rb, .rdoc, .md) are converted to .html paths
606612
# - Anchor URLs (#foo) pass through unchanged for GitHub-style header linking
607613
# - Footnote links get wrapped in <sup> tags
614+
# - URLs with an unsafe scheme (javascript:, vbscript:, data:, file:) are not
615+
# linked and only +text+ is returned
608616

609617
def gen_url(url, text)
610618
scheme, url, id = parse_url url
611619

620+
# Check the parsed URL because link: URLs are used as href without it
621+
return text if unsafe_url?(url)
622+
612623
if %w[http https link].include?(scheme) && url =~ /\.(gif|png|jpg|jpeg|bmp)\z/
613624
"<img src=\"#{url}\" />"
614625
else
@@ -629,6 +640,19 @@ def gen_url(url, text)
629640
end
630641
end
631642

643+
##
644+
# Does +url+ have a scheme in UNSAFE_URL_SCHEME_REGEXP?
645+
#
646+
# Browsers ignore leading C0 controls and spaces and all ASCII tabs and
647+
# newlines in a URL, so they are removed before checking the scheme.
648+
# See https://url.spec.whatwg.org/#concept-basic-url-parser
649+
#
650+
#: (String) -> bool
651+
def unsafe_url?(url)
652+
normalized = url.sub(/\A[\x00-\x20]+/, '').delete("\t\n\r")
653+
UNSAFE_URL_SCHEME_REGEXP.match?(normalized)
654+
end
655+
632656
##
633657
# Determines the HTML list element for +list_type+ and +open_tag+
634658

‎test/rdoc/markup/to_html_test.rb‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,8 @@ def test_convert_TIDYLINK_escape_text
931931
end
932932

933933
def test_convert_TIDYLINK_escape_javascript
934-
assert_not_include '{click}[javascript:alert`javascript_scheme`]', '<a href="javascript:'
934+
result = @to.convert '{click}[javascript:alert`javascript_scheme`]'
935+
assert_equal "\n<p>click</p>\n", result
935936
end
936937

937938
def test_convert_TIDYLINK_escape_onmouseover
@@ -959,6 +960,32 @@ def test_gen_url
959960
@to.gen_url('link:example', 'example')
960961
end
961962

963+
def test_gen_url_unsafe_scheme
964+
[
965+
'javascript:alert(1)',
966+
'JavaScript:alert(1)',
967+
" \x01javascript:alert(1)",
968+
"java\tscript:alert(1)",
969+
'vbscript:msgbox(1)',
970+
'data:text/html,alert(1)',
971+
'file:///etc/passwd',
972+
].each do |url|
973+
assert_equal 'example', @to.gen_url(url, 'example'), url.inspect
974+
end
975+
end
976+
977+
def test_gen_url_link_unsafe_scheme
978+
# The href is "javascript" on Windows because File.split treats
979+
# ":alert(1)" as an NTFS alternate data stream
980+
result = @to.gen_url('link:javascript:alert(1)', 'example')
981+
assert_not_include result, 'href="javascript:'
982+
end
983+
984+
def test_gen_url_scheme_like_path
985+
assert_equal '<a href="foo/javascript:alert">example</a>',
986+
@to.gen_url('foo/javascript:alert', 'example')
987+
end
988+
962989
def test_gen_url_rdoc_label
963990
assert_equal '<a href="#foottext-1">example</a>',
964991
@to.gen_url('rdoc-label:foottext-1', 'example')

0 commit comments

Comments
 (0)