Skip to content

Commit

Permalink
Fixes #36759 - only call allowed transpilers
Browse files Browse the repository at this point in the history
CVE-2022-3874: OS command injection via ct_command and fcct_command

Instead of allowing to call *any* command by changing a setting, only
allow specific paths to ct/fcct. If the user needs a different path,
they can set it via settings.yaml.
  • Loading branch information
evgeni committed Sep 20, 2023
1 parent 772cfd2 commit 362e8b2
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 16 deletions.
31 changes: 23 additions & 8 deletions app/registries/foreman/settings/provisioning.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

Foreman::SettingManager.define(:foreman) do
category(:provisioning, N_('Provisioning')) do
CT_LOCATIONS = %w(/usr/bin/ct /usr/local/bin/ct)
FCCT_LOCATIONS = %w(/usr/bin/fcct /usr/local/bin/fcct)

owner_select = proc do
[{:name => _("Users"), :class => 'user', :scope => 'visible', :value_method => 'id_and_type', :text_method => 'login'},
{:name => _("Usergroups"), :class => 'usergroup', :scope => 'visible', :value_method => 'id_and_type', :text_method => 'name'}]
Expand Down Expand Up @@ -119,16 +122,28 @@
description: N_("Default 'Host initial configuration' template, automatically assigned when a new operating system is created"),
default: 'Linux host_init_config default',
full_name: N_("Default 'Host initial configuration' template"))
setting('ct_command',
setting('ct_location',
type: :string,
description: N_("Full path to CoreOS transpiler (ct)"),
default: CT_LOCATIONS.first,
full_name: N_("CoreOS Transpiler Command"),
collection: proc { CT_LOCATIONS.zip(CT_LOCATIONS).to_h })
setting('ct_arguments',
type: :array,
description: N_("Full path to CoreOS transpiler (ct) with arguments as an comma-separated array"),
default: [which('ct'), '--pretty', '--files-dir', Rails.root.join('config', 'ct').to_s],
full_name: N_("CoreOS Transpiler Command"))
setting('fcct_command',
description: N_("CoreOS transpiler (ct) arguments as an comma-separated array"),
default: ['--pretty', '--files-dir', Rails.root.join('config', 'ct').to_s],
full_name: N_("CoreOS Transpiler Command Arguments"))
setting('fcct_location',
type: :string,
description: N_("Full path to Fedora CoreOS transpiler (fcct)"),
default: FCCT_LOCATIONS.first,
full_name: N_("Fedora CoreOS Transpiler Command"),
collection: proc { FCCT_LOCATIONS.zip(FCCT_LOCATIONS).to_h })
setting('fcct_arguments',
type: :array,
description: N_("Full path to Fedora CoreOS transpiler (fcct) with arguments as an comma-separated array"),
default: [which('fcct'), '--pretty', '--files-dir', Rails.root.join('config', 'ct').to_s],
full_name: N_("Fedora CoreOS Transpiler Command"))
description: N_("Fedora CoreOS transpiler (fcct) arguments as an comma-separated array"),
default: ['--pretty', '--files-dir', Rails.root.join('config', 'ct').to_s],
full_name: N_("Fedora CoreOS Transpiler Command Arguments"))

# We have following loop twice to keep the historical order.
# TODO: First resolve the correct order and then optimize this loop.
Expand Down
6 changes: 4 additions & 2 deletions app/services/foreman/renderer/scope/macros/transpilers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ module Transpilers
end
def transpile_coreos_linux_config(input, validate_input = true, validate_output = false)
YAML.safe_load(input) if validate_input
result = Foreman::CommandRunner.new(Setting[:ct_command], input).run!
ct_command = [Setting[:ct_location]] + Setting[:ct_arguments]
result = Foreman::CommandRunner.new(ct_command, input).run!
JSON.parse(result) if validate_output
result
end
Expand All @@ -29,7 +30,8 @@ def transpile_coreos_linux_config(input, validate_input = true, validate_output
example "transpile_coreos_linux_config(\"---\\nyaml: blah\") #=> JSON"
end
def transpile_fedora_coreos_config(input)
Foreman::CommandRunner.new(Setting[:fcct_command], input).run!
fcct_command = [Setting[:fcct_location]] + Setting[:fcct_arguments]
Foreman::CommandRunner.new(fcct_command, input).run!
end
end
end
Expand Down
23 changes: 23 additions & 0 deletions db/migrate/20230920093000_move_ct_command_into_own_setting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
class MoveCtCommandIntoOwnSetting < ActiveRecord::Migration[6.1]
def up
['ct', 'fcct'].each do |setting_prefix|
setting = Setting.find_by(name: "#{setting_prefix}_command")
if setting
setting.value = setting.value.drop(1)
setting.name = "#{setting_prefix}_arguments"
setting.save
end
end
end

def down
['ct', 'fcct'].each do |setting_prefix|
setting = Setting.find_by(name: "#{setting_prefix}_arguments")
if setting
setting.value = ["/usr/bin/#{setting_prefix}"] + setting.value
setting.name = "#{setting_prefix}_command"
setting.save
end
end
end
end
8 changes: 4 additions & 4 deletions test/fixtures/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ attributes66:
name: delivery_method
value: :test
attribute97:
name: ct_command
value: "['/usr/bin/cat']"
name: ct_location
value: "/usr/bin/true"
attribute98:
name: fcct_command
value: "['/usr/bin/cat']"
name: fcct_location
value: "/usr/bin/true"
4 changes: 2 additions & 2 deletions test/unit/foreman/renderer/scope/macros/transpilers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class TranspilersTest < ActiveSupport::TestCase

describe '#transpile_coreos_linux_config' do
test 'should call the transpiler' do
Foreman::CommandRunner.any_instance.expects(:capture3).with(Setting[:ct_command], "IGNORE")
Foreman::CommandRunner.any_instance.expects(:capture3).with([Setting[:ct_location]] + Setting[:ct_arguments], "IGNORE")
.returns(["JSON", "", @success])

assert_equal "JSON", @scope.transpile_coreos_linux_config("IGNORE")
Expand All @@ -28,7 +28,7 @@ class TranspilersTest < ActiveSupport::TestCase

describe '#transpile_fedora_coreos_config' do
test 'should call the transpiler' do
Foreman::CommandRunner.any_instance.expects(:capture3).with(Setting[:fcct_command], "IGNORE")
Foreman::CommandRunner.any_instance.expects(:capture3).with([Setting[:fcct_location]] + Setting[:fcct_arguments], "IGNORE")
.returns(["JSON", "", @success])

assert_equal "JSON", @scope.transpile_fedora_coreos_config("IGNORE")
Expand Down

0 comments on commit 362e8b2

Please sign in to comment.