Support Ruby 4.0

.github/workflows/tests.yml

@@ -5,7 +5,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        ruby: ['2.7', '3.0', '3.1', '3.2', '3.3', '3.4']
+        ruby: ['2.7', '3.0', '3.1', '3.2', '3.3', '3.4', '4.0']
       fail-fast: false
     steps:
       - name: Checkout

Rakefile

@@ -1,6 +1,5 @@
 require 'bundler/gem_tasks'
 require 'rake/testtask'
-require 'rdoc/task'
 
 desc 'Default: run unit tests.'
 task default: :test
@@ -13,14 +12,6 @@ Rake::TestTask.new(:test) do |t|
   t.warning = false
 end
 
-Rake::RDocTask.new(:rdoc) do |rdoc|
-  rdoc.rdoc_dir = 'rdoc'
-  rdoc.title    = 'Devise'
-  rdoc.options << '--line-numbers' << '--inline-source'
-  rdoc.rdoc_files.include('README.md')
-  rdoc.rdoc_files.include('lib/**/*.rb')
-end
-
 task :console do
   require 'irb'
   require 'irb/completion'

lib/ro_crate/model/directory.rb

@@ -74,9 +74,8 @@ def full_entry_path(relative_path)
     end
 
     def list_all_files(source_directory, include_hidden: false)
-      args = ['**/*']
-      args << ::File::FNM_DOTMATCH if include_hidden
-      Dir.chdir(source_directory) { Dir.glob(*args) }.reject do |path|
+      flags = include_hidden ? ::File::FNM_DOTMATCH : 0
+      Dir.glob('**/*', flags, base: source_directory).reject do |path|
         path == '.' || path == '..' || path.end_with?('/.')
       end
     end

lib/ro_crate/reader.rb

@@ -1,7 +1,11 @@
+require 'zip/version'
+
 module ROCrate
   ##
   # A class to handle reading of RO-Crates from Zip files or directories.
   class Reader
+    LEGACY_EXTRACT = Zip::VERSION.start_with?('2.').freeze
+
     ##
     # Reads an RO-Crate from a directory or zip file.
     #
@@ -42,15 +46,15 @@ def self.unzip_to(source, target)
     #
     # @param source [#read] An IO-like object containing a Zip file.
     # @param target [String, ::File, Pathname] The target directory where the file should be unzipped.
-    def self.unzip_io_to(io, target)
-      Dir.chdir(target) do
-        Zip::InputStream.open(io) do |input|
-          while (entry = input.get_next_entry)
-            unless ::File.exist?(entry.name) || entry.name_is_directory?
-              FileUtils::mkdir_p(::File.dirname(entry.name))
-              ::File.binwrite(entry.name, input.read)
-            end
-          end
+    def self.unzip_io_to(source, target)
+      target_path = Pathname(target)
+      Zip::InputStream.open(source) do |input|
+        while (entry = input.get_next_entry)
+          next if entry.name_is_directory?
+          dest = safe_join(target_path, entry.name)
+          next if dest.exist?
+          FileUtils.mkdir_p(dest.dirname)
+          ::File.binwrite(dest, input.read)
         end
       end
     end
@@ -60,15 +64,14 @@ def self.unzip_io_to(io, target)
     #
     # @param source [String, ::File, Pathname] The location of the zip file.
     # @param target [String, ::File, Pathname] The target directory where the file should be unzipped.
-    def self.unzip_file_to(file_or_path, target)
-      Dir.chdir(target) do
-        Zip::File.open(file_or_path) do |zipfile|
-          zipfile.each do |entry|
-            unless ::File.exist?(entry.name)
-              FileUtils::mkdir_p(::File.dirname(entry.name))
-              zipfile.extract(entry, entry.name)
-            end
-          end
+    def self.unzip_file_to(source, target)
+      target_path = Pathname(target)
+      Zip::File.open(source) do |zipfile|
+        zipfile.each do |entry|
+          dest = safe_join(target_path, entry.name)
+          next if dest.exist?
+          FileUtils.mkdir_p(dest.dirname)
+          LEGACY_EXTRACT ? entry.extract(dest) : entry.extract(entry.name, destination_directory: target_path)
         end
       end
     end
@@ -345,5 +348,28 @@ def self.detect_root_directory(source)
 
       nil
     end
+
+    ##
+    # Safely joins a desired file path onto a base directory, raising an exception if the path attempts to traverse
+    # outside it.
+    #
+    # @param base [Pathname] The base directory where the file will go.
+    # @param path [String] The desired file path.
+    #
+    # @raise [ROCrate::ReadException] Raised if an unsafe path is given.
+    #
+    # @return [Pathname] The safely joined base + path.
+    def self.safe_join(base, path)
+      dest = base.join(path)
+      # Guard against zip-slip attacks.
+      begin
+        unsafe = dest.expand_path.relative_path_from(base.expand_path).each_filename.first == '..'
+      rescue ArgumentError # Handle unjoinable paths, e.g. on different drives.
+        unsafe = true
+      end
+      raise ROCrate::ReadException, "Unsafe path in zip entry: #{path}" if unsafe
+
+      dest
+    end
   end
 end

ro_crate.gemspec

@@ -10,7 +10,7 @@ Gem::Specification.new do |s|
   s.licenses    = ['MIT']
   s.add_runtime_dependency 'addressable', '>= 2.7', '< 3'
   s.add_runtime_dependency 'rubyzip', '>= 2.3', '< 4'
-  s.add_development_dependency 'rake', '~> 13.0.0'
+  s.add_development_dependency 'rake', '~> 13.4.2'
   s.add_development_dependency 'test-unit', '~> 3.5.3'
   s.add_development_dependency 'simplecov', '~> 0.21.2'
   s.add_development_dependency 'yard', '~> 0.9.25'

test/reader_test.rb

@@ -386,19 +386,49 @@ class ReaderTest < Test::Unit::TestCase
     assert_equal 'a_file', data.first.name
   end
 
-  private
+  test 'protect against zip-slip' do
+    Dir.mktmpdir do |dir|
+      subdir = ::File.join(dir, 'subdir')
+      ::Dir.mkdir(subdir)
+
+      # Relative
+      e = check_exception(ROCrate::ReadException) do
+        ROCrate::Reader.unzip_file_to(fixture_file('unsafe/relative0.zip').path, subdir)
+      end
+      assert_include e.message, 'Unsafe path in zip entry: ../moo'
+      refute ::File.exist?(::File.join(dir, 'moo'))
+
+      e = check_exception(ROCrate::ReadException) do
+        ROCrate::Reader.unzip_io_to(fixture_file('unsafe/relative0.zip'), subdir)
+      end
+      assert_include e.message, 'Unsafe path in zip entry: ../moo'
+      refute ::File.exist?(::File.join(dir, 'moo'))
+
+      # Absolute
+      e = check_exception(ROCrate::ReadException) do
+        ROCrate::Reader.unzip_file_to(fixture_file('unsafe/absolute1.zip').path, subdir)
+      end
+      assert_include e.message, 'Unsafe path in zip entry: /tmp/moo'
 
-  def check_exception(exception_class)
-    e = nil
-    assert_raise(exception_class) do
+      e = check_exception(ROCrate::ReadException) do
+        ROCrate::Reader.unzip_io_to(fixture_file('unsafe/absolute1.zip'), subdir)
+      end
+      assert_include e.message, 'Unsafe path in zip entry: /tmp/moo'
+
+      # Simulate ArgumentError in safe_join
       begin
-        yield
-      rescue exception_class => e
-        raise e
+        original_expand_path = Pathname.instance_method(:expand_path)
+        Pathname.define_method(:expand_path) do |*args|
+          raise ArgumentError, 'Oh no'
+        end
+        e = check_exception(ROCrate::ReadException) do
+          ROCrate::Reader.unzip_file_to(fixture_file('unsafe/absolute1.zip').path, subdir)
+        end
+        assert_include e.message, 'Unsafe path in zip entry: /tmp/moo'
+      ensure
+        Pathname.define_method(:expand_path, original_expand_path)
       end
     end
-
-    e
   end
 
   test 'reads spec 1.1 RO-Crate and preserves version' do

test/test_helper.rb

@@ -5,10 +5,37 @@
 require 'ro_crate'
 require 'webmock/test_unit'
 
+class Test::Unit::TestCase
+  def teardown
+    self._opened_files.each do |f|
+      f.close unless f.closed?
+    end
+  end
+
+  def _opened_files
+    @opened_files ||= []
+  end
+end
+
 def fixture_file(name, *args)
-  ::File.open(::File.join(fixture_dir, name), *args)
+  f = ::File.open(::File.join(fixture_dir, name), *args)
+  self._opened_files << f
+  f
 end
 
 def fixture_dir
   ::File.join(::File.dirname(__FILE__), 'fixtures')
 end
+
+def check_exception(exception_class)
+  e = nil
+  assert_raise(exception_class) do
+    begin
+      yield
+    rescue exception_class => e
+      raise e
+    end
+  end
+
+  e
+end