Only escape HTML from breadcrumbs. Resolves #1658 (#1663)

This commit is contained in:
benjamin wil
2021-02-13 16:04:42 -08:00
committed by GitHub
parent 17162aa091
commit 5b7b9f40ae
4 changed files with 103 additions and 36 deletions
+2 -2
View File
@@ -26,9 +26,9 @@ module Precious
title = crumb.basename
if title == path.basename
breadcrumb << %{<li class="breadcrumb-item" aria-current="page">#{CGI.escape(title.to_s)}</li>}
breadcrumb << %{<li class="breadcrumb-item" aria-current="page">#{CGI.escapeHTML(title.to_s)}</li>}
else
breadcrumb << %{<li class="breadcrumb-item"><a href="#{overview_path}/#{crumb}/">#{CGI.escape(title.to_s)}</a></li>}
breadcrumb << %{<li class="breadcrumb-item"><a href="#{overview_path}/#{crumb}/">#{CGI.escapeHTML(title.to_s)}</a></li>}
end
end
breadcrumb << %{</ol></nav>}
+6 -7
View File
@@ -16,15 +16,10 @@ module Precious
DEFAULT_AUTHOR = 'you'
@@to_xml = { :save_with => Nokogiri::XML::Node::SaveOptions::DEFAULT_XHTML ^ 1, :indent => 0, :encoding => 'UTF-8' }
def title
h1 = @h1_title ? page_header_from_content(@content) : false
h1 || @page.url_path_title # url_path_title is the metadata title if present, otherwise the filename-based title
end
def page_header
title
end
def breadcrumb
path = Pathname.new(@page.url_path).parent
return '' if path.to_s == '.'
@@ -32,7 +27,7 @@ module Precious
path.descend do |crumb|
element = "#{crumb.basename}"
next if element == @page.title
breadcrumb << %{<li class="breadcrumb-item"><a href="#{overview_path}/#{crumb}/">#{CGI.escape(element.to_s)}</a></li>}
breadcrumb << %{<li class="breadcrumb-item"><a href="#{overview_path}/#{crumb}/">#{CGI.escapeHTML(element.to_s)}</a></li>}
end
breadcrumb << %{</ol></nav>}
breadcrumb.join("\n")
@@ -269,6 +264,10 @@ module Precious
result << "</tr>\n</table>\n"
end
def title
h1 = @h1_title ? page_header_from_content(@content) : false
h1 || @page.url_path_title # url_path_title is the metadata title if present, otherwise the filename-based title
end
end
end
end
+26 -5
View File
@@ -43,12 +43,22 @@ context "Precious::Views::Overview" do
@page.instance_variable_set("@base_url", "")
assert_equal "<nav aria-label=\"Breadcrumb\"><ol><li class=\"breadcrumb-item\"><a href=\"/gollum/overview\">Home</a></li>\n<li class=\"breadcrumb-item\"><a href=\"/gollum/overview/Mordor/\">Mordor</a></li>\n<li class=\"breadcrumb-item\"><a href=\"/gollum/overview/Mordor/Eye-Of-Sauron/\">Eye-Of-Sauron</a></li>\n<li class=\"breadcrumb-item\" aria-current=\"page\">Saruman</li>\n</ol></nav>", @page.breadcrumb
end
test 'guard against malicious filenames' do
malicious_title = '<img src=x onerror=alert(1) />'
@page.instance_variable_set("@path", malicious_title)
test "breadcrumbs guard against malicious filenames" do
malicious_path = '<script>alert("malicious-content");/Very Bad'
@page.instance_variable_set("@path", malicious_path)
@page.instance_variable_set("@base_url", "")
assert @page.breadcrumb.include?(">%3Cimg+src%3Dx+onerror%3Dalert%281%29+</a>")
refute_includes @page.breadcrumb, malicious_path
assert_includes @page.breadcrumb, ">&lt;script&gt;alert(&quot;malicious-content&quot;);</a>"
end
test "breadcrumbs retain unicode and ASCII characters" do
title = "数学 📘"
@page.instance_variable_set("@path", title)
@page.instance_variable_set("@base_url", "")
assert_includes @page.breadcrumb, title
end
test "breadcrumb with no path" do
@@ -79,6 +89,17 @@ context "Precious::Views::Overview" do
assert_equal result[:name], 'Orc'
end
test "files_folders retain unicode and ASCII characters" do
@page.instance_variable_set("@path", "Mordor")
@page.instance_variable_set("@base_url", "")
@page.instance_variable_set("@results", [
FakePageResult.new("Mordor/Eye-Of-Sauron-👁️-数学.md")
])
result = @page.files_folders.first
assert result[:name], "Eye Of Sauron 👁️ 数学"
end
test "base url" do
# based on test "files_folders"
@page.instance_variable_set("@path", "Mordor")
+69 -22
View File
@@ -13,15 +13,41 @@ context "Precious::Views::Page" do
FileUtils.rm_rf(@path)
end
test 'guard against malicious filenames' do
malicious_title = '<img src=x onerror=alert(1) />'
@wiki.write_page(malicious_title, :markdown, 'Is Bilbo a hobbit? Why certainly!')
page = @wiki.page(malicious_title)
test "breadcrumbs guard against malicious input" do
malicious_path = '<script>alert("malicious-content");/Very Bad'
@wiki.write_page(malicious_path, :markdown, 'Is Bilbo a hobbit? Why certainly!')
page = @wiki.page(malicious_path)
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, false
assert @view.breadcrumb.include?(">%3Cimg+src%3Dx+onerror%3Dalert%281%29+</a>")
refute_includes @view.breadcrumb, malicious_path
assert_includes @view.breadcrumb, ">&lt;script&gt;alert(&quot;malicious-content&quot;);</a>"
end
test "breadcrumbs retain unicode and ASCII characters" do
path = "数学 📘/Age of Bilbo"
@wiki.write_page(path, :markdown, "How old is Bilbo?")
page = @wiki.page(path)
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, false
assert_include @view.breadcrumb, "数学 📘"
end
test "page header retains unicde and ASCII characters" do
title = "数学 📘"
@wiki.write_page(title, :markdown, "How old is Bilbo?")
page = @wiki.page(title)
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, false
assert @view.page_header, "数学 📘"
end
test "h1 title sanitizes correctly" do
@@ -35,10 +61,46 @@ context "Precious::Views::Page" do
@view.instance_variable_set :@h1_title, true
# Test page_header_from_content(@content)
actual = @view.title
assert_equal '1 & 2', actual
assert @view.page_header, "1 & 2"
end
test "page header uses filename when h1_title is false" do
title = "H1"
contents = <<~TEXT
# First H1 header
# Second H1 header
TEXT
@wiki.write_page(title, :markdown, contents, commit_details)
page = @wiki.page(title)
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, false
assert_equal @view.page_header, "H1"
end
test "page header uses filename when h1_title is true" do
contents = <<~TEXT
# First H1 header
# Second H1 header
TEXT
@wiki.write_page("H1", :markdown, contents, commit_details)
page = @wiki.page("H1")
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, true
assert_equal @view.page_header, "First H1 header"
end
test "metadata is rendered into a table" do
title = 'metadata test'
@wiki.write_page(title, :markdown, "---\nsome: metadata\nhere: for you\n---\n# Some markdown\nIn this doc")
@@ -104,21 +166,6 @@ EOS
assert_equal "594e928cc5dcb6d833dfb86bb36076fd4a84eea7", @view.id
end
test "h1 title can be disabled" do
title = 'H1'
@wiki.write_page(title, :markdown, '# 1 & 2 <script>alert("js")</script>' + "\n # 3", commit_details)
page = @wiki.page(title)
@view = Precious::Views::Page.new
@view.instance_variable_set :@page, page
@view.instance_variable_set :@content, page.formatted_data
@view.instance_variable_set :@h1_title, false
# Title is based on file name when h1_title is false.
actual = @view.title
assert_equal title, actual
end
test "breadcrumbs" do
@wiki.write_page('subdir/BC Test 1', :markdown, 'Test', commit_details)
page = @wiki.page('subdir/BC Test 1')