(FACT-3198) Do not allow null bytes in names and values

When storing reports in puppetdb, postgres does not allow for the
null utf-8 byte. This change normalizes all strings for both fact
keys and values to disallow that null byte.

lib/facter/custom_facts/util/collection.rb

@@ -32,7 +32,7 @@ def define_fact(name, options = {}, &block)
 
         fact
       rescue StandardError => e
-        log.log_exception("Unable to add fact #{name}: #{e}")
+        log.log_exception(e)
       end
 
       # Add a resolution mechanism for a named fact.  This does not distinguish
@@ -48,6 +48,8 @@ def add(name, options = {}, &block)
         fact.add(options, &block)
 
         fact
+      rescue StandardError => e
+        log.log_exception(e)
       end
 
       include Enumerable

lib/facter/custom_facts/util/fact.rb

@@ -35,7 +35,7 @@ class Fact
       #
       # @api private
       def initialize(name, options = {})
-        @name = name.to_s.downcase.intern
+        @name = LegacyFacter::Util::Normalization.normalize(name.to_s.downcase).intern
 
         @options = options.dup
         extract_ldapname_option!(options)

lib/facter/custom_facts/util/normalization.rb

@@ -54,6 +54,10 @@ def normalize_string(value)
           raise NormalizationError, "String #{value.inspect} doesn't match the reported encoding #{value.encoding}"
         end
 
+        if value.codepoints.include?(0)
+          raise NormalizationError, "String #{value.inspect} contains a null byte reference; unsupported for all facts."
+        end
+
         value
       rescue EncodingError
         raise NormalizationError, "String encoding #{value.encoding} is not UTF-8 and could not be converted to UTF-8"

spec/custom_facts/util/collection_spec.rb

@@ -91,8 +91,19 @@
       expect(fact.ldapname).to eq 'NewFact'
     end
 
+    it 'logs an error if the fact name contains the utf-8 null byte' do
+      name = "Uncool\0Name"
+      normalization_error = LegacyFacter::Util::Normalization::NormalizationError
+      expect(logger).to receive(:log_exception).with(an_instance_of(normalization_error)) do |exception|
+        expect(exception.message).to match(/contains a null byte reference/)
+      end
+      collection.define_fact(name)
+    end
+
     it 'logs an error if the fact could not be defined' do
-      expect(logger).to receive(:log_exception).with('Unable to add fact newfact: kaboom!')
+      expect(logger).to receive(:log_exception).with(RuntimeError) do |exception|
+        expect(exception.message).to equal('kaboom!')
+      end
 
       collection.define_fact(:newfact) do
         raise 'kaboom!'

spec/custom_facts/util/fact_spec.rb

@@ -16,6 +16,12 @@
     expect { Facter::Util::Fact.new }.to raise_error(ArgumentError)
   end
 
+  it 'raises a normalization error when trying a name has a UTF-8 null byte' do
+    expect do
+      Facter::Util::Fact.new("Cool \0 name")
+    end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError)
+  end
+
   describe '#initialize' do
     it 'persists options' do
       fact = Facter::Util::Fact.new('yay', options)

spec/custom_facts/util/normalization_spec.rb

@@ -75,6 +75,19 @@ def utf8(str)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                            /String.*doesn't match the reported encoding UTF-8/)
       end
+
+      it 'rejects UTF-8 strings that contain null bytes' do
+        test_strings = ["\u0000",
+                        "\0",
+                        "\x00",
+                        "null byte \x00 in the middle"]
+        test_strings.each do |str|
+          expect do
+            normalization.normalize(str)
+          end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
+                             /String.*contains a null byte reference/)
+        end
+      end
     end
 
     describe 'and string encoding is not supported', unless: String.instance_methods.include?(:encoding) do

spec_integration/facter_spec.rb

@@ -55,6 +55,38 @@ def tmp_filename(tmp_filename)
     end
 
     context 'with custom facts' do
+      context 'with null bytes' do
+        let(:logger) { instance_spy(Facter::Log) }
+
+        normalization_error = LegacyFacter::Util::Normalization::NormalizationError
+
+        it 'logs an error when a fact keyname contains a null byte' do
+          facter_code = proc do
+            Facter.add("key\0bad_name") do
+              setcode { 'valid_value' }
+            end
+          end
+          facter_code.call do
+            expect(logger).to receive(:log_exception).with(an_instance_of(normalization_error)) do |exception|
+              expect(exception.message).to match(/contains a null byte reference/)
+            end
+          end
+        end
+
+        it 'logs an error when a fact value contains a null byte' do
+          facter_code = proc do
+            Facter.add('keyname') do
+              setcode { "bad\0value" }
+            end
+          end
+          facter_code.call do
+            expect(logger).to receive(:log_exception).with(an_instance_of(normalization_error)) do |exception|
+              expect(exception.message).to match(/contains a null byte reference/)
+            end
+          end
+        end
+      end
+
       context 'with array as value' do
         before do
           Facter.add('arr_fact') do
@@ -99,6 +131,27 @@ def tmp_filename(tmp_filename)
     end
 
     context 'with external facts' do
+      context 'with facts that have null bytes' do
+        before do
+          Facter.search_external([ext_facts_dir])
+          data = { "bad_key\u0000" => 'valid value',
+                   'valid_key' => "\x00" }
+          write_to_file(tmp_filename('bad_facts.yaml'), YAML.dump(data))
+        end
+
+        it 'shows errors in the logs when trying to read in the facts with bad keys' do
+          expect { Facter.value("bad_key\u0000") }.to raise_error(ArgumentError)
+        end
+
+        it 'shows errors in the logs when trying to read in the facts with bad values' do
+          allow(Facter).to receive(:log_exception) do |exception, message|
+            expect(exception).to be_a_kind_of(LegacyFacter::Util::Normalization::NormalizationError)
+            expect(message).to match(/contains a null byte reference/)
+          end
+          expect(Facter.value('valid_key')).to be_nil
+        end
+      end
+
       context 'with array as value' do
         before do
           Facter.search_external([ext_facts_dir])