Skip to content

Commit efe7aa6

Browse files
committed
Ensure we read from sanitised paths
1 parent 7b15eb5 commit efe7aa6

File tree

4 files changed

+56
-18
lines changed

4 files changed

+56
-18
lines changed

spec/example_app/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
This file is here only for the sake of a spec. It's not really a README.

spec/example_app/app/controllers/docs_controller.rb

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,16 @@ def show
2121
def render_page(name, title = nil)
2222
page = DocPage.find(name)
2323

24-
if page
25-
title = title || page.title
26-
@page_title = [title, "Administrate"].compact.join(" - ")
27-
# rubocop:disable Rails/OutputSafety
28-
render layout: "docs", html: page.body.html_safe
29-
# rubocop:enable Rails/OutputSafety
30-
else
31-
render file: Rails.root.join("public", "404.html"),
32-
layout: false,
33-
status: :not_found
34-
end
24+
title = title || page.title
25+
@page_title = [title, "Administrate"].compact.join(" - ")
26+
# rubocop:disable Rails/OutputSafety
27+
render layout: "docs", html: page.body.html_safe
28+
# rubocop:enable Rails/OutputSafety
29+
rescue DocPage::PageNotAllowed, DocPage::PageNotFound
30+
render(
31+
file: Rails.root.join("public", "404.html"),
32+
layout: false,
33+
status: :not_found,
34+
)
3535
end
3636
end

spec/example_app/app/models/doc_page.rb

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,43 @@
11
class DocPage
2+
class PageNotFound < StandardError
3+
def initialize(page)
4+
"Could not find page #{page.inspect}"
5+
end
6+
end
7+
8+
class PageNotAllowed < StandardError
9+
def initialize(page)
10+
"Page #{page.inspect} is not allowed"
11+
end
12+
end
13+
214
class << self
315
def find(page)
416
full_path = Rails.root + "../../#{page}.md"
17+
raise PageNotFound.new(page) unless path_exists?(full_path)
18+
19+
safe_path = filter_unsafe_paths(full_path)
20+
raise PageNotAllowed.new(page) unless safe_path
21+
22+
text = File.read(safe_path)
23+
new(text)
24+
end
25+
26+
private
27+
28+
def path_exists?(full_path)
29+
File.exist?(full_path)
30+
end
31+
32+
def doc_paths
33+
[
34+
Dir.glob(Rails.root + "../../**/*.md"),
35+
Dir.glob(Rails.root + "../../*.md"),
36+
].join
37+
end
538

6-
if File.exist?(full_path)
7-
text = File.read(full_path)
8-
new(text)
9-
end
39+
def filter_unsafe_paths(full_path)
40+
doc_paths[full_path.to_s]
1041
end
1142
end
1243

spec/models/doc_page_spec.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,16 @@
22

33
RSpec.describe DocPage do
44
describe ".find" do
5-
it "is nil if the page doesn't exist" do
6-
page = DocPage.find("not_a_page")
5+
it "raises an error if the page doesn't exist" do
6+
expect do
7+
DocPage.find("not_a_page")
8+
end.to raise_error(DocPage::PageNotFound)
9+
end
710

8-
expect(page).to be_nil
11+
it "raises an error on cheeky paths" do
12+
expect do
13+
DocPage.find("docs/../spec/example_app/README")
14+
end.to raise_error(DocPage::PageNotAllowed)
915
end
1016

1117
it "renders pages without metadata" do

0 commit comments

Comments
 (0)