From efe7aa6f73d0cfd0632939a64fb1f773d8834cdb 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

---
 spec/example_app/README.md                    |  1 +
 .../app/controllers/docs_controller.rb        | 22 +++++------
 spec/example_app/app/models/doc_page.rb       | 39 +++++++++++++++++--
 spec/models/doc_page_spec.rb                  | 12 ++++--
 4 files changed, 56 insertions(+), 18 deletions(-)
 create mode 100644 spec/example_app/README.md

diff --git a/spec/example_app/README.md b/spec/example_app/README.md
new file mode 100644
index 0000000000..8551b1d99b
--- /dev/null
+++ b/spec/example_app/README.md
@@ -0,0 +1 @@
+This file is here only for the sake of a spec. It's not really a README.
diff --git a/spec/example_app/app/controllers/docs_controller.rb b/spec/example_app/app/controllers/docs_controller.rb
index 08298a783c..78a61d8201 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..deb3255ce1 100644
--- a/spec/example_app/app/models/doc_page.rb
+++ b/spec/example_app/app/models/doc_page.rb
@@ -1,12 +1,43 @@
 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"
+      raise PageNotFound.new(page) unless path_exists?(full_path)
+
+      safe_path = filter_unsafe_paths(full_path)
+      raise PageNotAllowed.new(page) unless safe_path
+
+      text = File.read(safe_path)
+      new(text)
+    end
+
+    private
+
+    def path_exists?(full_path)
+      File.exist?(full_path)
+    end
+
+    def doc_paths
+      [
+        Dir.glob(Rails.root + "../../**/*.md"),
+        Dir.glob(Rails.root + "../../*.md"),
+      ].join
+    end
 
-      if File.exist?(full_path)
-        text = File.read(full_path)
-        new(text)
-      end
+    def filter_unsafe_paths(full_path)
+      doc_paths[full_path.to_s]
     end
   end
 
diff --git a/spec/models/doc_page_spec.rb b/spec/models/doc_page_spec.rb
index ff39898229..a583988e66 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 do
+        DocPage.find("not_a_page")
+      end.to raise_error(DocPage::PageNotFound)
+    end
 
-      expect(page).to be_nil
+    it "raises an error on cheeky paths" do
+      expect do
+        DocPage.find("docs/../spec/example_app/README")
+      end.to raise_error(DocPage::PageNotAllowed)
     end
 
     it "renders pages without metadata" do