diff --git a/lib/jekyll/page.rb b/lib/jekyll/page.rb index 79542d8b..1d9380b1 100644 --- a/lib/jekyll/page.rb +++ b/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 diff --git a/lib/jekyll/post.rb b/lib/jekyll/post.rb index 0c95cc7d..553c12cf 100644 --- a/lib/jekyll/post.rb +++ b/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 diff --git a/lib/jekyll/tags/include.rb b/lib/jekyll/tags/include.rb index adc7c2bf..aa608bc0 100644 --- a/lib/jekyll/tags/include.rb +++ b/lib/jekyll/tags/include.rb @@ -95,14 +95,13 @@ eos end def render(context) - dir = File.join(context.registers[:site].source, INCLUDES_DIR) - validate_dir(dir, context.registers[:site].safe) + dir = File.join(File.realpath(context.registers[:site].source), INCLUDES_DIR) 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) - if File.symlink?(dir) && safe - raise IOError.new "Includes directory '#{dir}' cannot be a symlink" + def validate_path(path, dir, safe) + if safe && !realpath_prefixed_with?(path, dir) + 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) - relative_file = Pathname.new(file).relative_path_from(Pathname.new(sourcedir)) - 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 path_relative_to_source(dir, path) + File.join(INCLUDES_DIR, path.sub(Regexp.new("^#{dir}"), "")) + end + + def realpath_prefixed_with?(path, dir) + File.exist?(path) && File.realpath(path).start_with?(dir) end def blank? diff --git a/lib/jekyll/url.rb b/lib/jekyll/url.rb index 10e1cb5d..813b9c87 100644 --- a/lib/jekyll/url.rb +++ b/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 diff --git a/test/source/_includes/tmp b/test/source/_includes/tmp new file mode 120000 index 00000000..cad23091 --- /dev/null +++ b/test/source/_includes/tmp @@ -0,0 +1 @@ +/tmp \ No newline at end of file diff --git a/test/source/_posts/2014-01-06-permalink-traversal.md b/test/source/_posts/2014-01-06-permalink-traversal.md new file mode 100644 index 00000000..c3f77d1b --- /dev/null +++ b/test/source/_posts/2014-01-06-permalink-traversal.md @@ -0,0 +1,5 @@ +--- +permalink: /%2e%2e/%2e%2e/%2e%2e/baddie.html +--- + +# Test diff --git a/test/source/exploit.md b/test/source/exploit.md new file mode 100644 index 00000000..c3f77d1b --- /dev/null +++ b/test/source/exploit.md @@ -0,0 +1,5 @@ +--- +permalink: /%2e%2e/%2e%2e/%2e%2e/baddie.html +--- + +# Test diff --git a/test/test_filters.rb b/test/test_filters.rb index d3f857a8..1959e6f9 100644 --- a/test/test_filters.rb +++ b/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 diff --git a/test/test_generated_site.rb b/test/test_generated_site.rb index a27477a2..ea20a15f 100644 --- a/test/test_generated_site.rb +++ b/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 diff --git a/test/test_page.rb b/test/test_page.rb index ef2f175b..8e60922f 100644 --- a/test/test_page.rb +++ b/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 diff --git a/test/test_post.rb b/test/test_post.rb index ee2eff6b..7d4ab599 100644 --- a/test/test_post.rb +++ b/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" diff --git a/test/test_site.rb b/test/test_site.rb index eace35f7..7e593c86 100644 --- a/test/test_site.rb +++ b/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 diff --git a/test/test_tags.rb b/test/test_tags.rb index 4874fda0..d7c8df24 100644 --- a/test/test_tags.rb +++ b/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 = < '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 = < '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 = <