CVE-2017-2298 (Ruby, published 2017-06-30T20:29Z)
Description: value: The mcollective-sshkey-security plugin before 0.5.1 for Puppet uses a server-specified identifier as part of a path where a file is written. A compromised server could use this to write a file to an arbitrary location on the client with the filename appended with the string "_pub.pem".
Severity: MEDIUM (CVSS v3 base score: 6.5)
Weakness: CWE-20 \u2014 Improper Input Validation
The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Repository: https://github.com/puppetlabs/mcollective-sshkey-security
Files changed:
security/sshkey.rb
spec/security/sshkey_spec.rb
Commit message: (MCOP-600) Prevent public key overwriting attack via identity

When using two-way automatic public key distribution, each end writes
the others `identity` as a public key file locally. No validation was
done on the `identity`, so it could trigger directory traversal and
allow the attacker to overwrite an unexpected file (like a trusted
public key certificate). Prevent this by verifying identity does not
result in traversing outside the intended distribution directory.


Vulnerable code:
          if File.exists?(old_keyfile = File.join(publickey_dir, "#{identity}_pub.pem"))
                File.open(File.join(publickey_dir, "#{identity}_pub.pem"), 'w') { |f| f.puts key }
            File.open(File.join(publickey_dir, "#{identity}_pub.pem"), 'w') { |f| f.puts key }
          File.stubs(:exists?).with('ssh/pkd/rspec_pub.pem').returns(false)
          File.expects(:open).with('ssh/pkd/rspec_pub.pem', 'w').yields(file)
          File.stubs(:exists?).with('ssh/pkd/rspec_pub.pem').returns(true)
          File.stubs(:read).with('ssh/pkd/rspec_pub.pem').returns('ssh-rsa dcba')
          File.stubs(:exists?).with('ssh/pkd/rspec_pub.pem').returns(true)
          File.stubs(:read).with('ssh/pkd/rspec_pub.pem').returns('ssh-rsa dcba')
          File.expects(:open).with('ssh/pkd/rspec_pub.pem', 'w').yields(file)

Fixed code:
          # Reject identity if it would result in directory traversal.
          old_keyfile = File.join(File.expand_path(publickey_dir), "#{identity}_pub.pem")
          unless File.expand_path(old_keyfile) == old_keyfile
            Log.warn("Identity returned by server would result in directory traversal. Not writing key to disk.")
            return
          end

          if File.exists?(old_keyfile)
                File.open(old_keyfile, 'w') { |f| f.puts key }
            File.open(old_keyfile, 'w') { |f| f.puts key }
        it 'should fail if identity would result in directory traversal' do
          @plugin.stubs(:lookup_config_option).with('learn_public_keys').returns('1')
          @plugin.stubs(:lookup_config_option).with('publickey_dir').returns('ssh/pkd')
          File.stubs(:directory?).with('ssh/pkd').returns(true)
          Log.expects(:warn)
          File.expects(:open).never
          @plugin.send(:write_key_to_disk, 'ssh-rsa abcd', '../test')
        end

          full_path = File.join(File.expand_path('ssh/pkd'), 'rspec_pub.pem')
          File.stubs(:exists?).with(full_path).returns(false)
          File.expects(:open).with(full_path, 'w').yields(file)
          full_path = File.join(File.expand_path('ssh/pkd'), 'rspec_pub.pem')
          File.stubs(:exists?).with(full_path).returns(true)
          File.stubs(:read).with(full_path).returns('ssh-rsa dcba')
          full_path = File.join(File.expand_path('ssh/pkd'), 'rspec_pub.pem')
          File.stubs(:exists?).with(full_path).returns(true)
          File.stubs(:read).with(full_path).returns('ssh-rsa dcba')
          File.expects(:open).with(full_path, 'w').yields(file)