From 2a679e797c0eee5864acdf8706d5b478fae86dc7 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Thu, 6 Sep 2018 12:30:59 -0400 Subject: [PATCH 1/7] Add failing tests for symlink check. --- .../symlinked-file-outside-source | 1 + test/test_entry_filter.rb | 11 +++++ test/test_layout_reader.rb | 41 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 120000 test/source/symlink-test/symlinked-file-outside-source diff --git a/test/source/symlink-test/symlinked-file-outside-source b/test/source/symlink-test/symlinked-file-outside-source new file mode 120000 index 00000000000..3594e94c04d --- /dev/null +++ b/test/source/symlink-test/symlinked-file-outside-source @@ -0,0 +1 @@ +/etc/passwd \ No newline at end of file diff --git a/test/test_entry_filter.rb b/test/test_entry_filter.rb index c9025092318..5dee7c63725 100644 --- a/test/test_entry_filter.rb +++ b/test/test_entry_filter.rb @@ -105,6 +105,17 @@ class TestEntryFilter < JekyllUnitTest refute_equal [], site.pages refute_equal [], site.static_files end + + should "include only safe symlinks in safe mode even when included" do + # no support for symlinks on Windows + skip_if_windows "Jekyll does not currently support symlinks on Windows." + + site = Site.new(site_configuration("safe" => true, "include" => ["symlinked-file-outside-source"])) + site.reader.read_directories("symlink-test") + + assert_equal %w(main.scss symlinked-file).length, site.pages.length + refute_includes site.static_files.map(&:name), "symlinked-file-outside-source" + end end context "#glob_include?" do diff --git a/test/test_layout_reader.rb b/test/test_layout_reader.rb index 96d2045a148..b15a1ca023f 100644 --- a/test/test_layout_reader.rb +++ b/test/test_layout_reader.rb @@ -31,5 +31,46 @@ class TestLayoutReader < JekyllUnitTest assert_equal LayoutReader.new(@site).layout_directory, source_dir("blah/_layouts") end end + + context "when a layout is a symlink" do + setup do + FileUtils.ln_sf("/etc/passwd", source_dir("_layouts", "symlink.html")) + @site.config = @site.config.merge({ + "safe" => true, + "include" => ["symlink.html"], + }) + end + + teardown do + FileUtils.rm(source_dir("_layouts", "symlink.html")) + end + + should "only read the layouts which are in the site" do + layouts = LayoutReader.new(@site).read + + refute layouts.keys.include?("symlink"), "Should not read the symlinked layout" + end + end + + context "with a theme" do + setup do + FileUtils.ln_sf("/etc/passwd", theme_dir("_layouts", "theme-symlink.html")) + @site.config = @site.config.merge({ + "include" => ["theme-symlink.html"], + "theme" => "test-theme", + "safe" => true, + }) + end + + teardown do + FileUtils.rm(theme_dir("_layouts", "theme-symlink.html")) + end + + should "not read a symlink'd theme" do + layouts = LayoutReader.new(@site).read + + refute layouts.keys.include?("theme-symlink"), "Should not read symlinked layout from theme" + end + end end end From a8b91de97b3196b3e0f282145f6bdefab1303bda Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Thu, 6 Sep 2018 13:11:41 -0400 Subject: [PATCH 2/7] EntryFilter#filter: reject all symlinks, even if included Previously, you could include the name of a symlinked file and Jekyll would not filter it. This is considered a bypass of the symlink checking, and thus a security bug. --- lib/jekyll/entry_filter.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/jekyll/entry_filter.rb b/lib/jekyll/entry_filter.rb index 019133195d3..822cc6530c5 100644 --- a/lib/jekyll/entry_filter.rb +++ b/lib/jekyll/entry_filter.rb @@ -31,9 +31,9 @@ def relative_to_source(entry) def filter(entries) entries.reject do |e| - unless included?(e) - special?(e) || backup?(e) || excluded?(e) || symlink?(e) - end + next true if symlink?(e) + next false if included?(e) + special?(e) || backup?(e) || excluded?(e) end end From 2c088e2596c9ad6bb15ded045adbde8da2c245f5 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 7 Sep 2018 12:29:04 -0400 Subject: [PATCH 3/7] Update tests for EntryFilter#filter fix and add comments for it --- lib/jekyll/entry_filter.rb | 3 +++ test/test_entry_filter.rb | 14 ++++++-------- test/test_layout_reader.rb | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/jekyll/entry_filter.rb b/lib/jekyll/entry_filter.rb index 822cc6530c5..000e4cff79b 100644 --- a/lib/jekyll/entry_filter.rb +++ b/lib/jekyll/entry_filter.rb @@ -31,8 +31,11 @@ def relative_to_source(entry) def filter(entries) entries.reject do |e| + # Reject this entry if it is a symlink. next true if symlink?(e) + # Do not reject this entry if it is included. next false if included?(e) + # Reject this entry if it is special, a backup file, or excluded. special?(e) || backup?(e) || excluded?(e) end end diff --git a/test/test_entry_filter.rb b/test/test_entry_filter.rb index 5dee7c63725..61d994cda56 100644 --- a/test/test_entry_filter.rb +++ b/test/test_entry_filter.rb @@ -5,7 +5,7 @@ class TestEntryFilter < JekyllUnitTest context "Filtering entries" do setup do - @site = Site.new(site_configuration) + @site = fixture_site end should "filter entries" do @@ -87,7 +87,7 @@ class TestEntryFilter < JekyllUnitTest # no support for symlinks on Windows skip_if_windows "Jekyll does not currently support symlinks on Windows." - site = Site.new(site_configuration("safe" => true)) + site = fixture_site("safe" => true) site.reader.read_directories("symlink-test") assert_equal %w(main.scss symlinked-file).length, site.pages.length @@ -99,18 +99,16 @@ class TestEntryFilter < JekyllUnitTest # no support for symlinks on Windows skip_if_windows "Jekyll does not currently support symlinks on Windows." - site = Site.new(site_configuration) - - site.reader.read_directories("symlink-test") - refute_equal [], site.pages - refute_equal [], site.static_files + @site.reader.read_directories("symlink-test") + refute_equal [], @site.pages + refute_equal [], @site.static_files end should "include only safe symlinks in safe mode even when included" do # no support for symlinks on Windows skip_if_windows "Jekyll does not currently support symlinks on Windows." - site = Site.new(site_configuration("safe" => true, "include" => ["symlinked-file-outside-source"])) + site = fixture_site("safe" => true, "include" => ["symlinked-file-outside-source"]) site.reader.read_directories("symlink-test") assert_equal %w(main.scss symlinked-file).length, site.pages.length diff --git a/test/test_layout_reader.rb b/test/test_layout_reader.rb index b15a1ca023f..cdf8f123457 100644 --- a/test/test_layout_reader.rb +++ b/test/test_layout_reader.rb @@ -48,7 +48,7 @@ class TestLayoutReader < JekyllUnitTest should "only read the layouts which are in the site" do layouts = LayoutReader.new(@site).read - refute layouts.keys.include?("symlink"), "Should not read the symlinked layout" + refute layouts.key?("symlink"), "Should not read the symlinked layout" end end @@ -69,7 +69,7 @@ class TestLayoutReader < JekyllUnitTest should "not read a symlink'd theme" do layouts = LayoutReader.new(@site).read - refute layouts.keys.include?("theme-symlink"), "Should not read symlinked layout from theme" + refute layouts.key?("theme-symlink"), "Should not read symlinked layout from theme" end end end From 35219a8c42e4853485ef10dc1bb8ef1766c6aca3 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 7 Sep 2018 12:35:30 -0400 Subject: [PATCH 4/7] Fix fmt errors. --- test/test_entry_filter.rb | 1 + test/test_layout_reader.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_entry_filter.rb b/test/test_entry_filter.rb index 61d994cda56..f41eed4f188 100644 --- a/test/test_entry_filter.rb +++ b/test/test_entry_filter.rb @@ -111,6 +111,7 @@ class TestEntryFilter < JekyllUnitTest site = fixture_site("safe" => true, "include" => ["symlinked-file-outside-source"]) site.reader.read_directories("symlink-test") + # rubocop:disable Performance/FixedSize assert_equal %w(main.scss symlinked-file).length, site.pages.length refute_includes site.static_files.map(&:name), "symlinked-file-outside-source" end diff --git a/test/test_layout_reader.rb b/test/test_layout_reader.rb index cdf8f123457..0f58365025d 100644 --- a/test/test_layout_reader.rb +++ b/test/test_layout_reader.rb @@ -69,7 +69,8 @@ class TestLayoutReader < JekyllUnitTest should "not read a symlink'd theme" do layouts = LayoutReader.new(@site).read - refute layouts.key?("theme-symlink"), "Should not read symlinked layout from theme" + refute layouts.key?("theme-symlink"), \ + "Should not read symlinked layout from theme" end end end From f5cd15cfd4094ba0632dbf2901d79479ff062494 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 7 Sep 2018 12:36:22 -0400 Subject: [PATCH 5/7] Run this branch. --- .travis.yml | 1 + appveyor.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 69fe263316f..84ca07419a7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,6 +32,7 @@ branches: - master - themes - /*-stable/ + - 3.7-entryfilter-symlink-fix notifications: slack: diff --git a/appveyor.yml b/appveyor.yml index 66c44d9f1eb..6ca69b35a93 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -6,6 +6,7 @@ branches: only: - master - themes + - 3.7-entryfilter-symlink-fix build: off From 7f1faea47d0d1665b32d075cbe6b5f0990f11577 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 7 Sep 2018 13:23:22 -0400 Subject: [PATCH 6/7] LayoutReader: skip tests if Windows --- test/test_layout_reader.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/test_layout_reader.rb b/test/test_layout_reader.rb index 0f58365025d..cade8d44f14 100644 --- a/test/test_layout_reader.rb +++ b/test/test_layout_reader.rb @@ -35,7 +35,7 @@ class TestLayoutReader < JekyllUnitTest context "when a layout is a symlink" do setup do FileUtils.ln_sf("/etc/passwd", source_dir("_layouts", "symlink.html")) - @site.config = @site.config.merge({ + @site = fixture_site({ "safe" => true, "include" => ["symlink.html"], }) @@ -46,6 +46,8 @@ class TestLayoutReader < JekyllUnitTest end should "only read the layouts which are in the site" do + skip_if_windows "Jekyll does not currently support symlinks on Windows." + layouts = LayoutReader.new(@site).read refute layouts.key?("symlink"), "Should not read the symlinked layout" @@ -55,7 +57,7 @@ class TestLayoutReader < JekyllUnitTest context "with a theme" do setup do FileUtils.ln_sf("/etc/passwd", theme_dir("_layouts", "theme-symlink.html")) - @site.config = @site.config.merge({ + @site = fixture_site({ "include" => ["theme-symlink.html"], "theme" => "test-theme", "safe" => true, @@ -67,6 +69,8 @@ class TestLayoutReader < JekyllUnitTest end should "not read a symlink'd theme" do + skip_if_windows "Jekyll does not currently support symlinks on Windows." + layouts = LayoutReader.new(@site).read refute layouts.key?("theme-symlink"), \ From 2025d1250239d0a8e96a1f5e5da6567cfc3d1c42 Mon Sep 17 00:00:00 2001 From: Parker Moore Date: Fri, 7 Sep 2018 13:28:00 -0400 Subject: [PATCH 7/7] Revert "Run this branch." This reverts commit f5cd15cfd4094ba0632dbf2901d79479ff062494. --- .travis.yml | 1 - appveyor.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 84ca07419a7..69fe263316f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,7 +32,6 @@ branches: - master - themes - /*-stable/ - - 3.7-entryfilter-symlink-fix notifications: slack: diff --git a/appveyor.yml b/appveyor.yml index 6ca69b35a93..66c44d9f1eb 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -6,7 +6,6 @@ branches: only: - master - themes - - 3.7-entryfilter-symlink-fix build: off