CVE-2016-3728: Improper Access Control (CWE-284)
value: Eval injection vulnerability in tftp_api.rb in the TFTP module in the Smart-Proxy in Foreman before 1.10.4 and 1.11.x before 1.11.2 allows remote attackers to execute arbitrary code via the PXE template type portion of the PATH_INFO to tftp/.
CWE description: The product does not restrict or incorrectly restricts access to a resource from an unauthorized actor.
CVSS v3 base score: 8.8
Published: 2016-05-20T14:59Z
Repository: https://github.com/theforeman/smart-proxy
Language: Ruby
Files changed: modules/tftp/tftp_api.rb, test/tftp/tftp_api_test.rb
Commit date: 2016-05-05 16:07:56 +0100
Commit message: Fixes #14931 - TFTP class instantiating fixed

Diff: diff --git a/modules/tftp/tftp_api.rb b/modules/tftp/tftp_api.rb
index 9218e8d..e2ff7f3 100644
--- a/modules/tftp/tftp_api.rb
+++ b/modules/tftp/tftp_api.rb
@@ -6,17 +6,18 @@ module Proxy::TFTP
     include ::Proxy::Log
     include ::Proxy::Validations
     helpers ::Proxy::Helpers
     authorize_with_trusted_hosts
     authorize_with_ssl_client
+    VARIANTS = ["Syslinux", "Pxegrub", "Pxegrub2", "Ztp", "Poap"].freeze
 
     helpers do
       def instantiate variant, mac=nil
         # Filenames must end in a hex representation of a mac address but only if mac is not empty
         log_halt 403, "Invalid MAC address: #{mac}"                  unless valid_mac?(mac) || mac.nil?
-        log_halt 403, "Unrecognized pxeboot config type: #{variant}" unless defined? variant.capitalize
-        eval "Proxy::TFTP::#{variant.capitalize}.new"
+        log_halt 403, "Unrecognized pxeboot config type: #{variant}" unless VARIANTS.include?(variant.capitalize)
+        Object.const_get("Proxy").const_get('TFTP').const_get(variant.capitalize).new
       end
 
       def create variant, mac
         tftp = instantiate variant, mac
         log_halt(400, "TFTP: Failed to create pxe config file: ") {tftp.set(mac, (params[:pxeconfig] || params[:syslinux_config]))}
diff --git a/test/tftp/tftp_api_test.rb b/test/tftp/tftp_api_test.rb
index 1dd5a16..2a0ff55 100644
--- a/test/tftp/tftp_api_test.rb
+++ b/test/tftp/tftp_api_test.rb
@@ -15,10 +15,21 @@ class TftpApiTest < Test::Unit::TestCase
   def setup
     Proxy::TFTP::Plugin.settings.stubs(:tftproot).returns("/some/root")
     @args = { :pxeconfig => "foo" }
   end
 
+  def test_instantiate_syslinux
+    obj = app.helpers.instantiate "syslinux", "AA:BB:CC:DD:EE:FF"
+    assert_equal "Proxy::TFTP::Syslinux", obj.class.name
+  end
+
+  def test_instantiate_nonexisting
+    subject = app
+    subject.helpers.expects(:log_halt).with(403, "Unrecognized pxeboot config type: Server").at_least(1)
+    subject.helpers.instantiate "Server", "AA:BB:CC:DD:EE:FF"
+  end
+
   def test_api_can_fetch_boot_file
     Proxy::Util::CommandTask.stubs(:new).returns(true)
     FileUtils.stubs(:mkdir_p).returns(true)
     Proxy::TFTP.expects(:fetch_boot_file).with('/some/root/boot/file','http://localhost/file').returns(true)
     post "/fetch_boot_file", :prefix => '/some/root/boot/file', :path => 'http://localhost/file'

Vulnerable code:         log_halt 403, "Unrecognized pxeboot config type: #{variant}" unless defined? variant.capitalize
        eval "Proxy::TFTP::#{variant.capitalize}.new"
Fixed code:     VARIANTS = ["Syslinux", "Pxegrub", "Pxegrub2", "Ztp", "Poap"].freeze
        log_halt 403, "Unrecognized pxeboot config type: #{variant}" unless VARIANTS.include?(variant.capitalize)
        Object.const_get("Proxy").const_get('TFTP').const_get(variant.capitalize).new
  def test_instantiate_syslinux
    obj = app.helpers.instantiate "syslinux", "AA:BB:CC:DD:EE:FF"
    assert_equal "Proxy::TFTP::Syslinux", obj.class.name
  end

  def test_instantiate_nonexisting
    subject = app
    subject.helpers.expects(:log_halt).with(403, "Unrecognized pxeboot config type: Server").at_least(1)
    subject.helpers.instantiate "Server", "AA:BB:CC:DD:EE:FF"
  end