From niloufar: security fix: DLL loading and extract path

ext/seven_zip_ruby/extconf.rb

@@ -140,11 +140,11 @@ def main
 
   if (RUBY_PLATFORM.include?("mswin"))
     # mswin32
-    $LIBS = "oleaut32.lib"
+    $LIBS = "oleaut32.lib shlwapi.lib"
     $CPPFLAGS = "/I.. /EHsc /DNDEBUG /DUSE_WIN32_FILE_API #{base_flag} #{$CPPFLAGS} "
   elsif (RUBY_PLATFORM.include?("mingw"))
     # MinGW
-    $LIBS = "-loleaut32 -static-libgcc -static-libstdc++"
+    $LIBS = "-loleaut32 -lshlwapi -static-libgcc -static-libstdc++"
 
     cpp0x_flag = [ "", "-std=gnu++11", "-std=c++11", "-std=gnu++0x", "-std=c++0x" ].find do |opt|
       try_compile(sample_cpp_source, "#{opt} -x c++ ")

ext/seven_zip_ruby/seven_zip_archive.cpp

@@ -317,7 +317,11 @@ VALUE ArchiveReader::open(VALUE in_stream, VALUE param)
 
     checkState(STATE_INITIAL, "Open error");
     if (ret != S_OK){
-        throw RubyCppUtil::RubyException("Invalid file format. open");
+        if (m_password_specified){
+            throw RubyCppUtil::RubyException("Invalid file format. open. or password is incorrect.");
+        }else{
+            throw RubyCppUtil::RubyException("Invalid file format. open.");
+        }
     }
 
     m_state = STATE_OPENED;
@@ -1761,15 +1765,44 @@ STDMETHODIMP OutStream::SetSize(UInt64 size)
 }
 
 
+#ifdef _WIN32
+#include "Shlwapi.h"
+static HINSTANCE gDllInstance = NULL;
+
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpReserved)
+{
+    // Perform actions based on the reason for calling.
+    switch( fdwReason ) 
+    { 
+        case DLL_PROCESS_ATTACH:
+            gDllInstance = hinstDLL;
+            break;
+        case DLL_PROCESS_DETACH:
+            gDllInstance = NULL;
+            break;
+    }
+    return TRUE;
+}
+#endif
+
 extern "C" void Init_seven_zip_archive(void)
 {
     using namespace SevenZip;
     using namespace RubyCppUtil;
 
 #ifdef _WIN32
-    gSevenZipHandle = LoadLibrary("./7z.dll");
+    WCHAR modulePath[MAX_PATH];
+    GetModuleFileNameW(gDllInstance, modulePath, _countof(modulePath));
+
+    SetDllDirectory("");
+
+    PathRemoveFileSpecW(modulePath);
+    PathAppendW(modulePath, L"7z.dll");
+    gSevenZipHandle = LoadLibraryW(modulePath);
     if (!gSevenZipHandle){
-        gSevenZipHandle = LoadLibrary("./7z64.dll");
+        PathRemoveFileSpecW(modulePath);
+        PathAppendW(modulePath, L"7z64.dll");
+        gSevenZipHandle = LoadLibraryW(modulePath);
     }
 #else
     gSevenZipHandle = dlopen("./7z.so", RTLD_NOW);

ext/seven_zip_ruby/utils.cpp

@@ -448,13 +448,13 @@ VALUE ConvertBstrToString(const BSTR &bstr)
     const int char_count = SysStringLen(bstr);
 #ifdef _WIN32
     const int len = WideCharToMultiByte(CP_UTF8, 0, bstr, char_count, NULL, 0, NULL, NULL);
-    VALUE str = rb_str_new(NULL, len);
+    VALUE str = rb_tainted_str_new(NULL, len);
 
     WideCharToMultiByte(CP_UTF8, 0, bstr, char_count, RSTRING_PTR(str), len, NULL, NULL);
 #else
     size_t len;
     Utf16_To_Utf8(NULL, &len, bstr, char_count);
-    VALUE str = rb_str_new(NULL, len);
+    VALUE str = rb_tainted_str_new(NULL, len);
     Utf16_To_Utf8(RSTRING_PTR(str), &len, bstr, char_count);
 #endif
     return str;

lib/seven_zip_ruby/seven_zip_reader.rb

@@ -462,13 +462,28 @@ def idx_prj.[](index)
 
     def file_proc(base_dir)  # :nodoc:
       base_dir = base_dir.to_s
+      base_dir = File.expand_path(base_dir)
       return Proc.new do |type, arg|
         case(type)
         when :stream
           ret = nil
           arg_path = Pathname(arg.path)
+          rp = arg_path.cleanpath
+          if "..#{File::SEPARATOR}" == rp.to_s[0..2]
+            raise InvalidArchive.new("#{arg.path} is Dangerous Path.")
+          end
           if (arg.anti?)
-            arg_path.rmtree if (arg_path.exist?)
+            pwd = Dir.pwd
+            Dir.chdir(base_dir)
+            rp = File.join(".", arg_path.to_s)
+            begin
+              if (File.exist?(rp))
+                require 'fileutils'
+                FileUtils.remove_entry_secure(rp)
+              end
+            ensure
+              Dir.chdir(pwd) rescue nil
+            end
           elsif (arg.file?)
             path = arg_path.expand_path(base_dir)
             path.parent.mkpath

test/res/The Flying Spaghetti Monster.txt

@@ -0,0 +1,3 @@
+宇宙は空飛ぶスパゲッティ・モンスターによって創造された。これは空飛ぶスパゲッティ・モンスターが大酒を飲んだ後の事であった。
+
+https://ja.wikipedia.org/wiki/%E7%A9%BA%E9%A3%9B%E3%81%B6%E3%82%B9%E3%83%91%E3%82%B2%E3%83%83%E3%83%86%E3%82%A3%E3%83%BB%E3%83%A2%E3%83%B3%E3%82%B9%E3%82%BF%E3%83%BC%E6%95%99

test/res/The Three Little Pigs.txt

@@ -0,0 +1,5 @@
+Little pig, little pig, let me come in.
+No, no, by the hair on my chinny chin chin.
+Then I'll huff, and I'll puff, and I'll blow your house in.
+
+https://en.wikipedia.org/wiki/The_Three_Little_Pigs

test/res/石肥三年.txt

@@ -0,0 +1,3 @@
+狐たちの間では、みんなが痛い目に遭っている彦一に一杯食わせようと企む奴は少なくない。ある狐は、夜のうちに彦一の家の畑に大量の石を投げ入れ、近くに隠れて様子をうかがっていた。翌朝になって畑にやって来た彦一は驚いたが、これが狐の仕業だと気付くと、わざと嬉しそうな顔をして、「こりゃ助かった。石の肥料は三年はもつ。これが馬の糞だったら大変な所じゃった」と、狐に聞こえるような大声で嘘をついた。これを真に受けた狐はその夜に石を全部取り除き、代わりに馬の糞をどっさり投げ入れた（馬の糞は良い肥料になる）。翌朝、彦一は「困った困った」と言いながらご機嫌な顔で畑を耕し、狐はこれで彦一を困らせてやったと満足して帰って行くのだった。
+
+https://ja.wikipedia.org/wiki/%E5%BD%A6%E4%B8%80

test/test_seven_zip_reader.rb

@@ -0,0 +1,187 @@
+# vim: tabstop=4 fileformat=unix fileencoding=utf-8 filetype=ruby
+
+require 'rubygems'
+# gem install test-unit
+require 'test/unit'
+
+STDERR.sync = true
+
+$basedir = File.dirname( $0 )
+$tmpdir = File.join( $basedir, "tmp" )
+$resourcedir = File.join( $basedir, "res" )
+
+#gem 'seven_zip_ruby_am', '< 1.2.6'
+require 'seven_zip_ruby'
+
+class TestSevenZipReader < Test::Unit::TestCase
+
+	def self.startup
+		self.remove_tmpdir
+	end
+
+	def self.shutdown
+		self.remove_tmpdir
+	end
+
+	def tmpdir
+		unless File.exist?( $tmpdir )
+			Dir.mkdir( $tmpdir ) rescue nil
+		end
+		$tmpdir
+	end
+
+	def self.remove_tmpdir
+		if File.exist?( $tmpdir )
+			Dir.rmdir( $tmpdir ) rescue nil
+		end
+	end
+
+	def remove_tmp_entries( entries )
+		entries.each do |f|
+			pth = File.join( $tmpdir, f )
+			File.delete( pth ) if File.exist?( pth )
+		end
+	end
+
+	def test_reader_extract_data
+		formats = [
+			["7ZIP", "7z", "It's a 7z!\r\n"], 
+		]
+		formats.each do |e|
+			type, ext, text, = *e
+			pth = File.join( $resourcedir, "test_reader_data.#{ext}" )
+			File.open( pth, "rb" ) do |file|
+				SevenZipRuby::Reader.open( file, :type => type ) do |szr|
+					first_file = szr.entries.select( &:file? ).first
+					data = szr.extract_data( first_file )
+					data.force_encoding( Encoding::UTF_8 )
+					assert_equal( text, data )
+				end
+			end
+		end
+	end
+
+	def test_reader_entries
+		pth = File.join( $resourcedir, "test_reader_files.7z" )
+		File.open( pth, "rb" ) do |file|
+			SevenZipRuby::Reader.open( file ) do |szr|
+				ent = szr.entries
+				assert_equal( "The Flying Spaghetti Monster.txt", ent[0].path )
+				assert_equal( "The Three Little Pigs.txt", ent[1].path )
+			end
+		end
+	end
+
+	def test_reader_extract
+		tmp = tmpdir()
+
+		entries = [
+			"The Flying Spaghetti Monster.txt", 
+			"The Three Little Pigs.txt"
+		]
+		remove_tmp_entries( entries )
+
+		pth = File.join( $resourcedir, "test_reader_files.7z" )
+		File.open( pth, "rb" ) do |file|
+			SevenZipRuby::Reader.open( file ) do |szr|
+				szr.extract_all( tmp )
+			end
+		end
+
+		entries.each do |f|
+			pth = File.join( tmp, f )
+			assert( File.exist?( pth ) )
+		end
+
+		remove_tmp_entries( entries )
+	end
+
+	def test_reader_filepath_encoding_cp932
+		tmp = tmpdir()
+
+		pth = File.join( $resourcedir, "test_reader_filename_cp932.7z" )
+		File.open( pth, "rb" ) do |file|
+			SevenZipRuby::Reader.open( file ) do |szr|
+				ent = szr.entries
+				assert_equal( "石肥三年.txt", ent[0].path )
+				szr.extract_all( tmp )
+			end
+		end
+		assert( File.exist?( File.join( tmp, "石肥三年.txt" ) ) )
+
+		remove_tmp_entries( ["石肥三年.txt"] )
+	end
+
+	def test_reader_extract_zs
+		data = <<EOS
+N3q8ryccAAM5UXek1gAAAAAAAACCAAAAAAAAAHAmzP/gAWcAzl0AcquM8ASZcZ9YsWJ8PeLq9S/f
+/B7sk0HmVfPeMRdIx/+HGN7+uZzlevM38ORzbn5op9BX8Kt+e2MOzGQbp4jp2XKAJZ9dP5lx4AyU
+QnwkqticRe2dhG+HqqPE2nI10yX6qSvK3HZsN9jHkAKOqF/MUk8T4iUHXKv7vgfaTWedWtjHO9Vo
+BMzMuGsh7Bbu5MaqZo9/FqQ6QzyG4y+KMnvsxN0lnkuDCfh6y3/1C/cMs8GsyAdDQsHq14eAh3YX
+AyB/0uTp7OxvM9J7k4d9UgAAAQQGAAEJgNYABwsBAAEhIQEIDIFoAAgKAYXmPCoAAAUBEUkALgAu
+AC8AVABoAGUAIABGAGwAeQBpAG4AZwAgAFMAcABhAGcAaABlAHQAdABpACAATQBvAG4AcwB0AGUA
+cgAuAHQAeAB0AAAAFAoBAA9NgCGjn9UBFQYBACAAAAAAAA==
+EOS
+		data, = *data.unpack( "m" )
+		file_name = 'The Flying Spaghetti Monster.txt'
+
+		tmp = tmpdir()
+		tmp_tmp = File.join( tmp, "tmp" )
+		Dir.mkdir( tmp_tmp ) rescue nil
+
+		file = StringIO.new( data, "rb" )
+			# szr.extract
+			safety = false
+			begin
+				SevenZipRuby::Reader.open( file ) do |szr|
+					ent = szr.entries
+					assert_equal( 1, ent.size )
+					assert_equal( "../#{file_name}", ent[0].path )
+
+					begin
+						szr.extract( ent[0].path, tmp_tmp )
+#						notify( "Vulnerable ?" )
+					rescue SevenZipRuby::InvalidArchive => err
+						assert_match( /Dangerous Path/i, err.message )
+						safety = true
+					end
+				end
+			rescue SevenZipRuby::InvalidOperation => err
+				# ignore
+			end
+			unless safety
+				notify( "The expected exception is not thrown." )
+			end
+			assert_path_not_exist( File.join( tmp, file_name ) )
+
+		file.rewind
+			# szr.extract_all
+			safety = false
+			begin
+				SevenZipRuby::Reader.open( file ) do |szr|
+					ent = szr.entries
+					begin
+						szr.extract_all( tmp_tmp )
+#						notify( "Vulnerable ?" )
+					rescue SevenZipRuby::InvalidArchive => err
+						assert_match( /Dangerous Path/i, err.message )
+						safety = true
+					end
+				end
+			rescue SevenZipRuby::InvalidOperation => err
+				# ignore
+			end
+			unless safety
+				notify( "The expected exception is not thrown." )
+			end
+			assert_path_not_exist( File.join( tmp, file_name ) )
+
+		file.close
+
+		Dir.rmdir( tmp_tmp ) if Dir.exist?( tmp_tmp )
+	end
+
+end
+
+
+