Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PUP-11772) Resolve Security cops main #9159

Merged
merged 2 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -918,23 +918,6 @@ Naming/VariableName:
Naming/VariableNumber:
Enabled: false

Security/Eval:
Exclude:
- 'lib/puppet/interface/action.rb'
- 'lib/puppet/interface/action_builder.rb'
- 'lib/puppet/pops/loader/ruby_data_type_instantiator.rb'
- '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:
Expand Down
2 changes: 1 addition & 1 deletion examples/enc/regexp_nodes/regexp_nodes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 =~ /^$/
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/file_system/file_impl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/file_system/posix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
6 changes: 4 additions & 2 deletions lib/puppet/interface/action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -265,12 +265,14 @@ def #{@name}(#{decl.join(", ")})
end
WRAPPER

# It should be possible to rewrite this code to use `define_method`
# instead of `class/instance_eval` since Ruby 1.8 is long dead.
if @face.is_a?(Class)
@face.class_eval do eval wrapper, nil, file, line end
@face.class_eval do eval wrapper, nil, file, line end # rubocop:disable Security/Eval
@face.send(:define_method, internal_name, &block)
@when_invoked = @face.instance_method(name)
else
@face.instance_eval do eval wrapper, nil, file, line end
@face.instance_eval do eval wrapper, nil, file, line end # rubocop:disable Security/Eval
@face.meta_def(internal_name, &block)
@when_invoked = @face.method(name).unbind
end
Expand Down
13 changes: 4 additions & 9 deletions lib/puppet/interface/action_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
# within the context of a new instance of this class.
# @api public
class Puppet::Interface::ActionBuilder
extend Forwardable

# The action under construction
# @return [Puppet::Interface::Action]
# @api private
Expand Down Expand Up @@ -142,15 +144,8 @@ def render_as(value = nil)
property = setter.to_s.chomp('=')

unless method_defined? property
# Using eval because the argument handling semantics are less awful than
# when we use the define_method/block version. The later warns on older
# Ruby versions if you pass the wrong number of arguments, but carries
# on, which is totally not what we want. --daniel 2011-04-18
eval <<-METHOD
def #{property}(value)
@action.#{property} = value
end
METHOD
# ActionBuilder#<property> delegates to Action#<setter>
def_delegator :@action, setter, property
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/loader/ruby_data_type_instantiator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
# make the private loader available in a binding to allow it to be passed on
loader_for_type = loader.private_loader
here = get_binding(loader_for_type)
created = eval(ruby_code_string, here, source_ref, 1)
created = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval
unless created.is_a?(Puppet::Pops::Types::PAnyType)
raise ArgumentError, _("The code loaded from %{source_ref} did not produce a data type when evaluated. Got '%{klass}'") % { source_ref: source_ref, klass: created.class }
end
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/pops/loader/ruby_function_instantiator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
# make the private loader available in a binding to allow it to be passed on
loader_for_function = loader.private_loader
here = get_binding(loader_for_function)
created = eval(ruby_code_string, here, source_ref, 1)
created = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval
unless created.is_a?(Class)
raise ArgumentError, _("The code loaded from %{source_ref} did not produce a Function class when evaluated. Got '%{klass}'") % { source_ref: source_ref, klass: created.class }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def self.create(loader, typed_name, source_ref, ruby_code_string)
# This will do the 3x loading and define the "function_<name>" and "real_function_<name>" methods
# in the anonymous module used to hold function definitions.
#
func_info = eval(ruby_code_string, here, source_ref, 1)
func_info = eval(ruby_code_string, here, source_ref, 1) # rubocop:disable Security/Eval

# Validate what was loaded
unless func_info.is_a?(Hash)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/provider/package/appdmg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/provider/package/windows/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/util/command_line/trollop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/puppet/util/execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/provider/package/appdmg_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down