Skip to content

Commit

Permalink
Ensure we read from sanitised paths
Browse files Browse the repository at this point in the history
  • Loading branch information
pablobm committed Aug 11, 2022
1 parent db6d1d4 commit f1265a9
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 19 deletions.
22 changes: 11 additions & 11 deletions spec/example_app/app/controllers/docs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ def show
def render_page(name, title = nil)
page = DocPage.find(name)

if page
title = title || page.title
@page_title = [title, "Administrate"].compact.join(" - ")
# rubocop:disable Rails/OutputSafety
render layout: "docs", html: page.body.html_safe
# rubocop:enable Rails/OutputSafety
else
render file: Rails.root.join("public", "404.html"),
layout: false,
status: :not_found
end
title = title || page.title
@page_title = [title, "Administrate"].compact.join(" - ")
# rubocop:disable Rails/OutputSafety
render layout: "docs", html: page.body.html_safe
# rubocop:enable Rails/OutputSafety
rescue DocPage::PageNotAllowed, DocPage::PageNotFound
render(
file: Rails.root.join("public", "404.html"),
layout: false,
status: :not_found
)
end
end
33 changes: 28 additions & 5 deletions spec/example_app/app/models/doc_page.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,35 @@
class DocPage
class PageNotFound < StandardError
def initialize(page)
"Could not find page #{page.inspect}"
end
end

class PageNotAllowed < StandardError
def initialize(page)
"Page #{page.inspect} is not allowed"
end
end

class << self
def find(page)
full_path = Rails.root + "../../#{page}.md"
full_path = (Rails.root + "../../#{page}.md").to_s

raise PageNotFound.new(page) unless File.exist?(full_path)
raise PageNotAllowed.new(page) unless safe_path?(full_path)

text = File.read(full_path)
new(text)
end

private

def doc_paths
Dir.glob(Rails.root + "../../**/*.md") + Dir.glob(Rails.root + "../../*.md")
end

if File.exist?(full_path)
text = File.read(full_path)
new(text)
end
def safe_path?(full_path)
doc_paths.include?(full_path)
end
end

Expand Down
12 changes: 9 additions & 3 deletions spec/models/doc_page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

RSpec.describe DocPage do
describe ".find" do
it "is nil if the page doesn't exist" do
page = DocPage.find("not_a_page")
it "raises an error if the page doesn't exist" do
expect {
DocPage.find("not_a_page")
}.to raise_error(DocPage::PageNotFound)
end

expect(page).to be_nil
it "raises an error on cheeky paths" do
expect {
DocPage.find("docs/../spec/example_app/README")
}.to raise_error(DocPage::PageNotAllowed)
end

it "renders pages without metadata" do
Expand Down

0 comments on commit f1265a9

Please sign in to comment.