From eec1c193d9043622bf27e162dfb8ffb248ae0caa Mon Sep 17 00:00:00 2001 From: Jacob Helwig Date: Thu, 12 Jul 2018 14:07:56 -0700 Subject: (MODULE-7443) Safely deserialize stringified array This ports PUP-8974, and the related follow-up maintenance commits from the Puppet repo. The augeas provider used Kernel#eval to convert stringified arrays to Ruby arrays. For example, it extracted the array part of the "clause" below: onlyif => 'values HostKey == ["/etc/ssh/ssh_host_rsa_key"]' and called Kernel#eval with '["/etc/ssh/ssh_host_rsa_key"]'. Using eval is bad because it executes arbitrary code. This commit changes the provider to convert the comma delimited string to a Ruby array. This mostly maintains the functionality of the original Kernel#eval (minus running arbitrary code) except for no longer handling the \M-x, \M-\C-x, \M-\cx, \c\M-x, \c?, and \C-? escape sequences in double-quoted strings, and \u{nnnn ...} is more lenient about whitespace. --- lib/puppet/provider/augeas/augeas.rb | 180 ++++++++++++++++++++++++++++++- spec/unit/provider/augeas/augeas_spec.rb | 65 +++++++++++ 2 files changed, 243 insertions(+), 2 deletions(-) diff --git a/lib/puppet/provider/augeas/augeas.rb b/lib/puppet/provider/augeas/augeas.rb index 05183e5..b64b0b3 100644 --- a/lib/puppet/provider/augeas/augeas.rb +++ b/lib/puppet/provider/augeas/augeas.rb @@ -18,7 +18,6 @@ require 'strscan' require 'puppet/util' require 'puppet/util/diff' require 'puppet/util/package' -require 'json' Puppet::Type.type(:augeas).provide(:augeas) do include Puppet::Util @@ -574,7 +573,184 @@ Puppet::Type.type(:augeas).provide(:augeas) do # rubocop:enable Style/GuardClause def to_array(string) - JSON.parse(string.tr("'", '"')) + s = StringScanner.new(string) + match = array_open(s) + raise "Unable to parse array. Unexpected character at: #{s.rest}" if match.nil? + + array_content = array_values(s) + + match = array_close(s) + raise "Unable to parse array. Unexpected character at: #{s.rest}" if match.nil? + + array_content end private :to_array + + def array_open(scanner) + scanner.scan(%r{\s*\[\s*}) + end + private :array_open + + def array_close(scanner) + scanner.scan(%r{\s*\]\s*}) + end + private :array_close + + def array_separator(scanner) + scanner.scan(%r{\s*,\s*}) + end + private :array_separator + + def single_quote_unescaped_char(scanner) + scanner.scan(%r{[^'\\]}) + end + private :single_quote_unescaped_char + + def single_quote_escaped_char(scanner) + scanner.scan(%r{\\(['\\])}) && scanner[1] + end + private :single_quote_escaped_char + + def single_quote_char(scanner) + single_quote_escaped_char(scanner) || single_quote_unescaped_char(scanner) + end + private :single_quote_char + + def double_quote_unescaped_char(scanner) + scanner.scan(%r{[^"\\]}) + end + private :double_quote_unescaped_char + + # This handles the possible Ruby escape sequences in double-quoted strings, + # except for \M-x, \M-\C-x, \M-\cx, \c\M-x, \c?, and \C-?. The full list of + # escape sequences, and their meanings is taken from: + # https://github.com/ruby/ruby/blob/90fdfec11a4a42653722e2ce2a672d6e87a57b8e/doc/syntax/literals.rdoc#strings + def double_quote_escaped_char(scanner) + match = scanner.scan(%r{\\(["\\abtnvfres0-7xu])}) + return nil if match.nil? + + case scanner[1] + when '\\' then return '\\' + when '"' then return '"' + when 'a' then return "\a" + when 'b' then return "\b" + when 't' then return "\t" + when 'n' then return "\n" + when 'v' then return "\v" + when 'f' then return "\f" + when 'r' then return "\r" + when 'e' then return "\e" + when 's' then return "\s" + when %r{[0-7]} + octal_character = scanner[1] + other_digits = scanner.scan(%r{[0-7]{1,2}}) + octal_character << other_digits unless other_digits.nil? + + return octal_character.to_i(8).chr + when 'x' + hex_character = scanner.scan(%r{[0-9a-fA-F]{1,2}}) + return nil if hex_character.nil? + + hex_character.to_i(16).chr + when 'u' + return unicode_short_hex_character(scanner) || unicode_long_hex_characters(scanner) + else + # Not a valid escape sequence as far as we're concerned. + return nil + end + end + private :double_quote_escaped_char + + def unicode_short_hex_character(scanner) + unicode_character = scanner.scan(%r{[0-9a-fA-F]{4}}) + return nil if unicode_character.nil? + + [unicode_character.hex].pack 'U' + end + private :unicode_short_hex_character + + def unicode_long_hex_characters(scanner) + unicode_string = '' + return nil unless scanner.scan(%r|{|) + + loop do + char = scanner.scan(%r{[0-9a-fA-F]{1,6}}) + break if char.nil? + unicode_string << [char.hex].pack('U') + + separator = scanner.scan(%r{\s}) + break if separator.nil? + end + + return nil if scanner.scan(%r|}|).nil? || unicode_string.empty? + + unicode_string + end + private :unicode_long_hex_characters + + def single_quoted_string(scanner) + quoted_string = '' + + match = scanner.scan(%r{'}) + return nil if match.nil? + + loop do + match = single_quote_char(scanner) + break if match.nil? + + quoted_string << match + end + + match = scanner.scan(%r{'}) + return quoted_string if match + + nil + end + private :single_quoted_string + + def double_quote_char(scanner) + double_quote_escaped_char(scanner) || double_quote_unescaped_char(scanner) + end + private :double_quote_char + + def double_quoted_string(scanner) + quoted_string = '' + + match = scanner.scan(%r{"}) + return nil if match.nil? + + loop do + match = double_quote_char(scanner) + break if match.nil? + + quoted_string << match + end + + match = scanner.scan(%r{"}) + return quoted_string if match + + nil + end + private :double_quoted_string + + def quoted_string(scanner) + single_quoted_string(scanner) || double_quoted_string(scanner) + end + private :quoted_string + + def array_values(scanner) + values = [] + + loop do + match = quoted_string(scanner) + break if match.nil? + values << match + + match = array_separator(scanner) + break if match.nil? + end + + values + end + private :array_values end diff --git a/spec/unit/provider/augeas/augeas_spec.rb b/spec/unit/provider/augeas/augeas_spec.rb index 6166140..180f89c 100644 --- a/spec/unit/provider/augeas/augeas_spec.rb +++ b/spec/unit/provider/augeas/augeas_spec.rb @@ -262,6 +262,49 @@ describe Puppet::Type.type(:augeas).provider(:augeas) do command = ['values', 'fake value', "== ['set', 'of', 'values']"] expect(provider.process_values(command)).to eq(true) end + it 'returns true for an array match with double quotes and spaces' do + command = ['values', 'fake value', '== [ "set" , "of" , "values" ] '] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with internally escaped single quotes' do + provider.aug.stubs(:match).returns(['set', "o'values", 'here']) + provider.aug.stubs(:get).returns('set').then.returns("o'values").then.returns('here') + command = ['values', 'fake value', "== [ 'set', 'o\\'values', 'here']"] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with octal character sequences' do + command = ['values', 'fake value', '== ["\\x73et", "of", "values"]'] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with hex character sequences' do + command = ['values', 'fake value', '== ["\\163et", "of", "values"]'] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with short unicode escape sequences' do + command = ['values', 'fake value', '== ["\\u0073et", "of", "values"]'] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with single character long unicode escape sequences' do + command = ['values', 'fake value', '== ["\\u{0073}et", "of", "values"]'] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with multi-character long unicode escape sequences' do + command = ['values', 'fake value', '== ["\\u{0073 0065 0074}", "of", "values"]'] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an array match with literal backslashes' do + provider.aug.stubs(:match).returns(['set', 'o\\values', 'here']) + provider.aug.stubs(:get).returns('set').then.returns('o\\values').then.returns('here') + command = ['values', 'fake value', '== [ "set", "o\\\\values", "here"]'] + expect(provider.process_values(command)).to eq(true) + end it 'returns false for an array non match' do command = ['values', 'fake value', "== ['this', 'should', 'not', 'match']"] @@ -277,6 +320,18 @@ describe Puppet::Type.type(:augeas).provider(:augeas) do command = ['values', 'fake value', "!= ['this', 'should', 'not', 'match']"] expect(provider.process_values(command)).to eq(true) end + + it 'returns true for an array non match with double quotes and spaces' do + command = ['values', 'fake value', '!= [ "this" , "should" ,"not", "match" ] '] + expect(provider.process_values(command)).to eq(true) + end + + it 'returns true for an empty array match' do + provider.aug.stubs(:match).returns([]) + provider.aug.stubs(:get) + command = ['values', 'fake value', '== []'] + expect(provider.process_values(command)).to eq(true) + end end describe 'match filters' do @@ -322,6 +377,11 @@ describe Puppet::Type.type(:augeas).provider(:augeas) do expect(provider.process_match(command)).to eq(true) end + it 'returns true for an array match with double quotes and spaces' do + command = ['match', 'fake value', '== [ "set" , "of" , "values" ] '] + expect(provider.process_match(command)).to eq(true) + end + it 'returns false for an array non match' do command = ['match', 'fake value', "== ['this', 'should', 'not', 'match']"] expect(provider.process_match(command)).to eq(false) @@ -336,6 +396,11 @@ describe Puppet::Type.type(:augeas).provider(:augeas) do command = ['match', 'fake value', "!= ['this', 'should', 'not', 'match']"] expect(provider.process_match(command)).to eq(true) end + + it 'returns true for an array non match with double quotes and spaces' do + command = ['match', 'fake value', '!= [ "this" , "should" ,"not", "match" ] '] + expect(provider.process_match(command)).to eq(true) + end end describe 'need to run' do -- cgit v1.2.3