Merge pull request #1946 from jekyll/security-vuln-patches

lib/jekyll/page.rb

@@ -135,7 +135,7 @@ module Jekyll
     #
     # Returns the destination file path String.
     def destination(dest)
-      path = File.join(dest, self.url)
+      path = File.join(dest, File.expand_path(self.url, "/"))
       path = File.join(path, "index.html") if self.url =~ /\/$/
       path
     end

lib/jekyll/post.rb

@@ -260,7 +260,7 @@ module Jekyll
     # Returns destination file path String.
     def destination(dest)
       # The url needs to be unescaped in order to preserve the correct filename
-      path = File.join(dest, CGI.unescape(self.url))
+      path = File.join(dest, File.expand_path(CGI.unescape(self.url), "/"))
       path = File.join(path, "index.html") if path[/\.html$/].nil?
       path
     end

lib/jekyll/tags/include.rb

@@ -95,14 +95,13 @@ eos
       end
 
       def render(context)
-        dir = File.join(context.registers[:site].source, INCLUDES_DIR)
+        dir = File.join(File.realpath(context.registers[:site].source), INCLUDES_DIR)
-        validate_dir(dir, context.registers[:site].safe)
 
         file = render_variable(context) || @file
         validate_file_name(file)
 
         path = File.join(dir, file)
-        validate_file(context.registers[:site].source, path, context.registers[:site].safe)
+        validate_path(path, dir, context.registers[:site].safe)
 
         begin
           partial = Liquid::Template.parse(source(path, context))
@@ -116,19 +115,20 @@ eos
         end
       end
 
-      def validate_dir(dir, safe)
+      def validate_path(path, dir, safe)
-        if File.symlink?(dir) && safe
+        if safe && !realpath_prefixed_with?(path, dir)
-          raise IOError.new "Includes directory '#{dir}' cannot be a symlink"
+          raise IOError.new "The included file '#{path}' should exist and should not be a symlink"
+        elsif !File.exist?(path)
+          raise IOError.new "Included file '#{path_relative_to_source(dir, path)}' not found"
         end
       end
 
-      def validate_file(sourcedir, file, safe)
+      def path_relative_to_source(dir, path)
-        relative_file = Pathname.new(file).relative_path_from(Pathname.new(sourcedir))
+        File.join(INCLUDES_DIR, path.sub(Regexp.new("^#{dir}"), ""))
-        if !File.exists?(file)
-          raise IOError.new "Included file '#{relative_file}' not found" 
-        elsif File.symlink?(file) && safe
-          raise IOError.new "The included file '#{relative_file}' should not be a symlink"
       end
+
+      def realpath_prefixed_with?(path, dir)
+        File.exist?(path) && File.realpath(path).start_with?(dir)
       end
 
       def blank?

lib/jekyll/url.rb

@@ -50,6 +50,7 @@ module Jekyll
 
     # Returns a sanitized String URL
     def sanitize_url(in_url)
+
       # Remove all double slashes
       url = in_url.gsub(/\/\//, "/")
 
@@ -61,6 +62,7 @@ module Jekyll
 
       # Always add a leading slash
       url.gsub!(/\A([^\/])/, '/\1')
+
       url
     end
   end

test/source/_includes/tmp

@@ -0,0 +1 @@
+/tmp

test/source/_posts/2014-01-06-permalink-traversal.md

@@ -0,0 +1,5 @@
+---
+permalink: /%2e%2e/%2e%2e/%2e%2e/baddie.html
+---
+
+# Test

test/source/exploit.md

@@ -0,0 +1,5 @@
+---
+permalink: /%2e%2e/%2e%2e/%2e%2e/baddie.html
+---
+
+# Test

test/test_filters.rb

@@ -131,7 +131,7 @@ class TestFilters < Test::Unit::TestCase
             assert_equal 2, g["items"].size
           when ""
             assert g["items"].is_a?(Array), "The list of grouped items for '' is not an Array."
-            assert_equal 8, g["items"].size
+            assert_equal 9, g["items"].size
           end
         end
       end

test/test_generated_site.rb

@@ -14,7 +14,7 @@ class TestGeneratedSite < Test::Unit::TestCase
     end
 
     should "ensure post count is as expected" do
-      assert_equal 38, @site.posts.size
+      assert_equal 39, @site.posts.size
     end
 
     should "insert site.posts into the index" do

test/test_page.rb

@@ -129,6 +129,16 @@ class TestPage < Test::Unit::TestCase
         assert_equal @page.permalink, @page.url
         assert_equal "/about/", @page.dir
       end
+
+      should "not be writable outside of destination" do
+        unexpected = File.expand_path("../../../baddie.html", dest_dir)
+        File.delete unexpected if File.exist?(unexpected)
+        page = setup_page("exploit.md")
+        do_render(page)
+        page.write(dest_dir)
+
+        assert !File.exist?(unexpected)
+      end
     end
 
     context "with specified layout of nil" do

test/test_post.rb

@@ -103,6 +103,17 @@ class TestPost < Test::Unit::TestCase
         assert_equal "/my_category/permalinked-post", @post.url
       end
 
+      should "not be writable outside of destination" do
+        unexpected = File.expand_path("../../../baddie.html", dest_dir)
+        File.delete unexpected if File.exist?(unexpected)
+        post = setup_post("2014-01-06-permalink-traversal.md")
+        do_render(post)
+        post.write(dest_dir)
+
+        assert !File.exist?(unexpected)
+        assert File.exist?(File.expand_path("baddie.html", dest_dir))
+      end
+
       context "with CRLF linebreaks" do
         setup do
           @real_file = "2009-05-24-yaml-linebreak.markdown"

test/test_site.rb

@@ -158,7 +158,7 @@ class TestSite < Test::Unit::TestCase
       stub.proxy(Dir).entries { |entries| entries.reverse }
       @site.process
       # files in symlinked directories may appear twice
-      sorted_pages = %w(.htaccess about.html bar.html coffeescript.coffee contacts.html deal.with.dots.html foo.md index.html index.html main.scss main.scss properties.html sitemap.xml symlinked-file)
+      sorted_pages = %w(.htaccess about.html bar.html coffeescript.coffee contacts.html deal.with.dots.html exploit.md foo.md index.html index.html main.scss main.scss properties.html sitemap.xml symlinked-file)
       assert_equal sorted_pages, @site.pages.map(&:name)
     end
 

test/test_tags.rb

@@ -363,6 +363,41 @@ CONTENT
   end
 
   context "include tag with parameters" do
+
+    context "with symlink'd include" do
+
+      should "not allow symlink includes" do
+        File.open("/tmp/pages-test", 'w') { |file| file.write("SYMLINK TEST") }
+        assert_raise IOError do
+          content = <<CONTENT
+---
+title: Include symlink
+---
+
+{% include tmp/pages-test %}
+
+CONTENT
+          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true, 'safe' => true })
+        end
+        assert_no_match /SYMLINK TEST/, @result
+      end
+
+      should "not expose the existence of symlinked files" do
+        ex = assert_raise IOError do
+          content = <<CONTENT
+---
+title: Include symlink
+---
+
+{% include tmp/pages-test-does-not-exist %}
+
+CONTENT
+          create_post(content, {'permalink' => 'pretty', 'source' => source_dir, 'destination' => dest_dir, 'read_posts' => true, 'safe' => true })
+        end
+        assert_match /should exist and should not be a symlink/, ex.message
+      end
+    end
+
     context "with one parameter" do
       setup do
         content = <<CONTENT