From f1265a97c507a2a53671a7ffaad7aaf3765a67a9 Mon Sep 17 00:00:00 2001
From: Pablo Brasero <pablo@pablobm.com>
Date: Thu, 11 Aug 2022 15:27:15 +0100
Subject: [PATCH] Ensure we read from sanitised paths

---
 .../app/controllers/docs_controller.rb        | 22 ++++++-------
 spec/example_app/app/models/doc_page.rb       | 33 ++++++++++++++++---
 spec/models/doc_page_spec.rb                  | 12 +++++--
 3 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/spec/example_app/app/controllers/docs_controller.rb b/spec/example_app/app/controllers/docs_controller.rb
index 08298a783c..de7b6f738d 100644
--- a/spec/example_app/app/controllers/docs_controller.rb
+++ b/spec/example_app/app/controllers/docs_controller.rb
@@ -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
diff --git a/spec/example_app/app/models/doc_page.rb b/spec/example_app/app/models/doc_page.rb
index 4e318c65cf..11dbb9bd36 100644
--- a/spec/example_app/app/models/doc_page.rb
+++ b/spec/example_app/app/models/doc_page.rb
@@ -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
 
diff --git a/spec/models/doc_page_spec.rb b/spec/models/doc_page_spec.rb
index ff39898229..837b26c8cf 100644
--- a/spec/models/doc_page_spec.rb
+++ b/spec/models/doc_page_spec.rb
@@ -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