From 283ba4c1088ac7bff16dce7a4042d0e9168a9b83 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Wed, 21 Oct 2020 20:21:37 -0700 Subject: [PATCH] (PUP-11772) Resolve Security/Open When opening a file path, use File.open When opening a URL, use URI.parse(..).open The Windows package class includes our Registry module which defines `open`. Use the fully qualified name to avoid rubocop confusion. --- .rubocop_todo.yml | 9 --------- examples/enc/regexp_nodes/regexp_nodes.rb | 2 +- lib/puppet/file_system/file_impl.rb | 2 +- lib/puppet/file_system/posix.rb | 2 +- lib/puppet/provider/package/appdmg.rb | 2 +- lib/puppet/provider/package/windows/package.rb | 4 ++-- lib/puppet/util/command_line/trollop.rb | 2 +- lib/puppet/util/execution.rb | 3 ++- spec/unit/provider/package/appdmg_spec.rb | 2 +- 9 files changed, 10 insertions(+), 18 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bca9a074090..c1a1f67dcc5 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -926,15 +926,6 @@ Security/Eval: - 'lib/puppet/pops/loader/ruby_function_instantiator.rb' - 'lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb' -Security/Open: - Exclude: - - 'lib/puppet/file_system/file_impl.rb' - - 'lib/puppet/file_system/posix.rb' - - 'lib/puppet/provider/package/appdmg.rb' - - 'lib/puppet/provider/package/windows/package.rb' - - 'lib/puppet/util/command_line/trollop.rb' - - 'lib/puppet/util/execution.rb' - # Configuration parameters: EnforcedStyle, AllowModifiersOnSymbols. # SupportedStyles: inline, group Style/AccessModifierDeclarations: diff --git a/examples/enc/regexp_nodes/regexp_nodes.rb b/examples/enc/regexp_nodes/regexp_nodes.rb index 971ba3769e2..b14d97d79ac 100644 --- a/examples/enc/regexp_nodes/regexp_nodes.rb +++ b/examples/enc/regexp_nodes/regexp_nodes.rb @@ -133,7 +133,7 @@ def matched_in_patternfile?(filepath, matchthis) patternlist = [] begin - open(filepath).each do |l| + File.open(filepath).each do |l| l.chomp! next if l =~ /^$/ diff --git a/lib/puppet/file_system/file_impl.rb b/lib/puppet/file_system/file_impl.rb index 75fb8f55be5..857289c7204 100644 --- a/lib/puppet/file_system/file_impl.rb +++ b/lib/puppet/file_system/file_impl.rb @@ -151,7 +151,7 @@ def lstat(path) end def compare_stream(path, stream) - open(path, 0, 'rb') { |this| FileUtils.compare_stream(this, stream) } + ::File.open(path, 0, 'rb') { |this| FileUtils.compare_stream(this, stream) } end def chmod(mode, path) diff --git a/lib/puppet/file_system/posix.rb b/lib/puppet/file_system/posix.rb index 12ab90e9fe5..7f690df6154 100644 --- a/lib/puppet/file_system/posix.rb +++ b/lib/puppet/file_system/posix.rb @@ -11,7 +11,7 @@ def binread(path) # issue this method reimplements the faster 2.0 version that will correctly # compare binary File and StringIO streams. def compare_stream(path, stream) - open(path, 0, 'rb') do |this| + ::File.open(path, 'rb') do |this| bsize = stream_blksize(this, stream) sa = String.new.force_encoding('ASCII-8BIT') sb = String.new.force_encoding('ASCII-8BIT') diff --git a/lib/puppet/provider/package/appdmg.rb b/lib/puppet/provider/package/appdmg.rb index e0f253a18cd..48e32377245 100644 --- a/lib/puppet/provider/package/appdmg.rb +++ b/lib/puppet/provider/package/appdmg.rb @@ -67,7 +67,7 @@ def self.installpkgdmg(source, name) end end - open(cached_source) do |dmg| + File.open(cached_source) do |dmg| xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-mountrandom", "/tmp", dmg.path ptable = Puppet::Util::Plist::parse_plist(xml_str) # JJM Filter out all mount-paths into a single array, discard the rest. diff --git a/lib/puppet/provider/package/windows/package.rb b/lib/puppet/provider/package/windows/package.rb index 3dff8b9e792..4ad019f73e6 100644 --- a/lib/puppet/provider/package/windows/package.rb +++ b/lib/puppet/provider/package/windows/package.rb @@ -44,9 +44,9 @@ def self.with_key(&block) [KEY64, KEY32].each do |mode| mode |= KEY_READ begin - open(hive, 'Software\Microsoft\Windows\CurrentVersion\Uninstall', mode) do |uninstall| + self.open(hive, 'Software\Microsoft\Windows\CurrentVersion\Uninstall', mode) do |uninstall| each_key(uninstall) do |name, wtime| - open(hive, "#{uninstall.keyname}\\#{name}", mode) do |key| + self.open(hive, "#{uninstall.keyname}\\#{name}", mode) do |key| yield key, values_by_name(key, reg_value_names_to_load) end end diff --git a/lib/puppet/util/command_line/trollop.rb b/lib/puppet/util/command_line/trollop.rb index 303e3a9d96c..60f754c246a 100644 --- a/lib/puppet/util/command_line/trollop.rb +++ b/lib/puppet/util/command_line/trollop.rb @@ -650,7 +650,7 @@ def parse_io_parameter param, arg else require 'open-uri' begin - open param + URI.parse(param).open rescue SystemCallError => e raise CommandlineError, _("file or url for option '%{arg}' cannot be opened: %{value0}") % { arg: arg, value0: e.message }, e.backtrace end diff --git a/lib/puppet/util/execution.rb b/lib/puppet/util/execution.rb index 34cccaf4ae7..aad07a333bf 100644 --- a/lib/puppet/util/execution.rb +++ b/lib/puppet/util/execution.rb @@ -78,7 +78,8 @@ def self.execpipe(command, failonfail = true) # a predictable output english_env = ENV.to_hash.merge( {'LANG' => 'C', 'LC_ALL' => 'C'} ) output = Puppet::Util.withenv(english_env) do - open("| #{command_str} 2>&1") do |pipe| + # We are intentionally using 'pipe' with open to launch a process + open("| #{command_str} 2>&1") do |pipe| # rubocop:disable Security/Open yield pipe end end diff --git a/spec/unit/provider/package/appdmg_spec.rb b/spec/unit/provider/package/appdmg_spec.rb index de6b4354a9e..34504a40df2 100644 --- a/spec/unit/provider/package/appdmg_spec.rb +++ b/spec/unit/provider/package/appdmg_spec.rb @@ -11,7 +11,7 @@ before do fh = double('filehandle', path: '/tmp/foo') resource[:source] = "foo.dmg" - allow(described_class).to receive(:open).and_yield(fh) + allow(File).to receive(:open).and_yield(fh) allow(Dir).to receive(:mktmpdir).and_return("/tmp/testtmp123") allow(FileUtils).to receive(:remove_entry_secure) end