From dfaae7be579e9a16ee895980bee21189b77d9362 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 20 Dec 2018 09:44:38 +0100 Subject: [PATCH 1/3] shell hardening --- src/clients/fcoe-client_auto.rb | 44 +++++++++++++------------- src/clients/fcoe-client_finish.rb | 29 ++++++----------- src/clients/inst_fcoe-client.rb | 2 +- src/include/fcoe-client/complex.rb | 45 ++++++++++++--------------- src/modules/FcoeClient.rb | 50 +++++++++++------------------- 5 files changed, 70 insertions(+), 100 deletions(-) diff --git a/src/clients/fcoe-client_auto.rb b/src/clients/fcoe-client_auto.rb index b1db377..7c97bba 100644 --- a/src/clients/fcoe-client_auto.rb +++ b/src/clients/fcoe-client_auto.rb @@ -23,18 +23,20 @@ # Package: Configuration of fcoe-client # Summary: Client for autoinstallation # Authors: Gabriele Mohr -# -# -# This is a client for autoinstallation. It takes its arguments, -# goes through the configuration and return the setting. -# Does not do any changes to the configuration. -# @param function to execute -# @param map/list of fcoe-client settings -# @return [Hash] edited settings, Summary or boolean on success depending on called function -# @example map mm = $[ "FAIL_DELAY" : "77" ]; -# @example map ret = WFM::CallFunction ("fcoe-client_auto", [ "Summary", mm ]); +require "shellwords" + module Yast + # + # This is a client for autoinstallation. It takes its arguments, + # goes through the configuration and return the setting. + # Does not do any changes to the configuration. + + # @param function to execute + # @param map/list of fcoe-client settings + # @return [Hash] edited settings, Summary or boolean on success depending on called function + # @example map mm = $[ "FAIL_DELAY" : "77" ]; + # @example map ret = WFM::CallFunction ("fcoe-client_auto", [ "Summary", mm ]); class FcoeClientAutoClient < Client def main Yast.import "UI" @@ -153,9 +155,9 @@ def main dev_name = card["dev_name"] if card["fcoe_vlan"] == FcoeClient.NOT_CONFIGURED if card["auto_vlan"] == "yes" - command = "fipvlan -c -s -f '-fcoe' #{dev_name}" + command = "/usr/sbin/fipvlan -c -s -f '-fcoe' #{dev_name.shellescape}" else - command = "fipvlan -c -s #{dev_name}" + command = "/usr/sbin/fipvlan -c -s #{dev_name.shellescape}" end ifcfg_file = Builtins.sformat( @@ -167,29 +169,25 @@ def main # call 'ifup' for the interface (creates /proc/net/vlan/) if FileUtils.Exists(ifcfg_file) cmd_ifup = Builtins.sformat( - "ifup %1.%2", - Ops.get_string(card, "dev_name", ""), - Ops.get_string(card, "vlan_interface", "") + "/usr/sbin/ifup %1.%2", + Ops.get_string(card, "dev_name", "").shellescape, + Ops.get_string(card, "vlan_interface", "").shellescape ) Builtins.y2milestone("Executing command: %1", cmd_ifup) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), cmd_ifup) - ) + output = SCR.Execute(path(".target.bash_output"), cmd_ifup) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) == 0 # start FCoE command = Builtins.sformat( - "fipvlan -s %1", - Ops.get_string(card, "dev_name", "") + "/usr/sbin/fipvlan -s %1", + Ops.get_string(card, "dev_name", "").shellescape ) end end Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) != 0 diff --git a/src/clients/fcoe-client_finish.rb b/src/clients/fcoe-client_finish.rb index 07a8373..a06233d 100644 --- a/src/clients/fcoe-client_finish.rb +++ b/src/clients/fcoe-client_finish.rb @@ -28,6 +28,8 @@ # Authors: # Gabriele Mohr # +require "shellwords" + require "yast2/systemd/socket" module Yast @@ -76,21 +78,10 @@ def main if @netcards != [] Builtins.y2milestone("Copying files /etc/fcoe/* to destination") # copy fcoe config files to destdir + destdir = File.join(Installation.destdir, "etc/fcoe") WFM.Execute( path(".local.bash"), - Ops.add( - Ops.add( - Ops.add( - Ops.add( - "test -d /etc/fcoe/ && mkdir -p '", - String.Quote(Installation.destdir) - ), - "/etc/fcoe' && cp -a /etc/fcoe/* '" - ), - String.Quote(Installation.destdir) - ), - "/etc/fcoe/'" - ) + "/usr/bin/test -d /etc/fcoe/ && /usr/bin/mkdir -p #{destdir.shellescape} && /usr/bin/cp -a /etc/fcoe/* #{destdir.shellescape}" ) else Builtins.y2milestone("Nothing to do") @@ -111,9 +102,9 @@ def main Ops.get_string(card, "vlan_interface", "") ) command = Builtins.sformat( - "cp -a %1 '%2/etc/sysconfig/network'", - file_name, - String.Quote(Installation.destdir) + "/usr/bin/cp -a %1 %2/etc/sysconfig/network", + file_name.shellescape, + Installation.destdir.shellescape ) Builtins.y2milestone("Executing command: %1", command) WFM.Execute(path(".local.bash"), command) @@ -123,9 +114,9 @@ def main Ops.get_string(card, "dev_name", "") ) command = Builtins.sformat( - "cp -a %1 '%2/etc/sysconfig/network'", - file_name, - String.Quote(Installation.destdir) + "/usr/bin/cp -a %1 %2/etc/sysconfig/network", + file_name.shellescape, + Installation.destdir.shellescape ) Builtins.y2milestone("Executing command: %1", command) WFM.Execute(path(".local.bash"), command) diff --git a/src/clients/inst_fcoe-client.rb b/src/clients/inst_fcoe-client.rb index 6fa7102..336c7dd 100644 --- a/src/clients/inst_fcoe-client.rb +++ b/src/clients/inst_fcoe-client.rb @@ -59,7 +59,7 @@ def main Builtins.y2milestone("fcoe-client module started during installation") # create /etc/fcoe - SCR.Execute(path(".target.bash"), "mkdir -p /etc/fcoe") + SCR.Execute(path(".target.bash"), "/usr/bin/mkdir -p /etc/fcoe") # FcoeClient::CheckInstalledPackages() not needed in inst-sys # FcoeClient::DetectStartStatus() doesn't make sense in inst-sys diff --git a/src/include/fcoe-client/complex.rb b/src/include/fcoe-client/complex.rb index 016765f..ad54e34 100644 --- a/src/include/fcoe-client/complex.rb +++ b/src/include/fcoe-client/complex.rb @@ -24,6 +24,9 @@ # Summary: Dialogs definitions # Authors: Gabriele Mohr # + +require "shellwords" + module Yast module FcoeClientComplexInclude def initialize_fcoe_client_complex(include_target) @@ -393,9 +396,9 @@ def HandleInterfacesDialog(id, event) end if card["auto_vlan"] == "yes" || vlan_interface == "0" - command = "fipvlan -c -s -f '-fcoe' #{dev_name}" + command = "/usr/sbin/fipvlan -c -s -f '-fcoe' #{dev_name.shellescape}" else - command = "fipvlan -c -s #{dev_name}" + command = "/usr/sbin/fipvlan -c -s #{dev_name.shellescape}" end output = {} @@ -420,9 +423,7 @@ def HandleInterfacesDialog(id, event) # execute command, e.g. 'fipvlan -c -s eth3' Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) != 0 @@ -439,7 +440,7 @@ def HandleInterfacesDialog(id, event) # if /etc/sysconfig/network/ifcfg-. already exists # call 'ifup' for the interface (creates /proc/net/vlan/.) if FileUtils.Exists(ifcfg_file) - cmd_ifup = Builtins.sformat("ifup %1.%2", dev_name, vlan_interface) + cmd_ifup = Builtins.sformat("/usr/sbin/ifup %1.%2", dev_name.shellescape, vlan_interface.shellescape) Builtins.y2milestone("Executing command: %1", cmd_ifup) output = Convert.to_map( SCR.Execute(path(".target.bash_output"), cmd_ifup) @@ -448,14 +449,12 @@ def HandleInterfacesDialog(id, event) if Ops.get_integer(output, "exit", 255) == 0 # only start FCoE - command = Builtins.sformat("fipvlan -s %1", dev_name) + command = Builtins.sformat("/usr/sbin/fipvlan -s %1", dev_name.shellescape) end end Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) != 0 if !FcoeClient.TestMode @@ -505,7 +504,7 @@ def HandleInterfacesDialog(id, event) # and removes the interface properly (tested on SP2 RC1) # TODO: Retest for SLES12 FcoeClient.AddRevertCommand( - Builtins.sformat("vconfig rem %1", fcoe_vlan_interface) + Builtins.sformat("/usr/sbin/vconfig rem %1", fcoe_vlan_interface.shellescape) ) else fcoe_vlan_interface = FcoeClient.NOT_CONFIGURED @@ -605,19 +604,17 @@ def HandleInterfacesDialog(id, event) # call fcoeadm -d first (bnc #719443) command = Builtins.sformat( - "fcoeadm -d %1", - Ops.get_string(card, "cfg_device", "") + "/usr/sbin/fcoeadm -d %1", + Ops.get_string(card, "cfg_device", "").shellescape ) Builtins.y2milestone("Calling %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) == 0 || FcoeClient.TestMode command = Builtins.sformat( - "vconfig rem %1", - Ops.get_string(card, "fcoe_vlan", "") + "/usr/sbin/vconfig rem %1", + Ops.get_string(card, "fcoe_vlan", "").shellescape ) Builtins.y2milestone("Calling %1", command) output = Convert.to_map( @@ -652,13 +649,11 @@ def HandleInterfacesDialog(id, event) if del_cfg command = Builtins.sformat( - "rm /etc/fcoe/cfg-%1", - Ops.get_string(card, "cfg_device", "") + "/usr/bin/rm /etc/fcoe/cfg-%1", + Ops.get_string(card, "cfg_device", "").shellescape ) Builtins.y2milestone("Calling %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) else Builtins.y2milestone( @@ -671,8 +666,8 @@ def HandleInterfacesDialog(id, event) if Ops.get_string(card, "vlan_interface", "") != "0" command = Builtins.sformat( - "rm /etc/sysconfig/network/ifcfg-%1", - Ops.get_string(card, "fcoe_vlan", "") + "/usr/bin/rm /etc/sysconfig/network/ifcfg-%1", + Ops.get_string(card, "fcoe_vlan", "").shellescape ) Builtins.y2milestone("Calling %1", command) output = Convert.to_map( diff --git a/src/modules/FcoeClient.rb b/src/modules/FcoeClient.rb index d998bea..899ad40 100644 --- a/src/modules/FcoeClient.rb +++ b/src/modules/FcoeClient.rb @@ -27,6 +27,9 @@ # # Representation of the configuration of fcoe-client. # Input and output routines. + +require "shellwords" + require "yast" require "yast2/systemd/socket" @@ -100,17 +103,6 @@ def main # return boolean return true if abort @AbortFunction = fun_ref(method(:Modified), "boolean ()") - # from IscsiClientLib.ycp (line 53) - reading output - # - # string from_bios = ((map)SCR::Execute(.target.bash_output, "iscsiadm -m fw"))["stdout"]:""; - # foreach(string row, splitstring(from_bios, "\n"), { - # list key_val=splitstring(row, "="); - # // if (size(key_val[0]:"")>0) ibft[key_val[0]:""] = key_val[1]:""; - # string kv = String::CutBlanks(key_val[0]:""); - # if (size(kv) > 0) ibft[kv] = String::CutBlanks(key_val[1]:""); - # }); - - # Define all the variables necessary to hold @current_card = 0 # currently selected card, means row in list of cards @@ -461,15 +453,13 @@ def CheckInstalledPackages def GetFcoeInfo(net_devices) # Add option -u (or --link_up): don't shut down interfaces # to be able to detect DCB state afterwards (see bnc #737683) - vlan_cmd = "LANG=POSIX fipvlan -u" + vlan_cmd = "LANG=POSIX /usr/sbin/fipvlan -u" if !Mode.autoinst - vlan_cmd = Ops.add(Ops.add(vlan_cmd, " -l "), @number_of_retries) + vlan_cmd << " -l #{@number_of_retries.to_s.shellescape}" end # reduce number of retries - Builtins.foreach( - Convert.convert(net_devices, :from => "list", :to => "list ") - ) { |dev| vlan_cmd = Ops.add(Ops.add(vlan_cmd, " "), dev) } + Builtins.foreach(net_devices) { |dev| vlan_cmd << " #{dev.shellescape}" } # call fipvlan command for all interfaces (saves time because is executed in parallel) Builtins.y2milestone("Executing command: %1", vlan_cmd) @@ -590,9 +580,9 @@ def GetFcoeVlanInterface(interface, vlan_interface) end command = Builtins.sformat( - "sed -n 's/\\([^ ]*\\) *.*%1*.*%2/\\1/p' /proc/net/vlan/config", - vlan_interface, - interface + "/usr/bin/sed -n 's/\\([^ ]*\\) *.*'%1'*.*'%2'/\\1/p' /proc/net/vlan/config", + vlan_interface.shellescape, + interface.shellescape ) Builtins.y2milestone("Executing command: %1", command) @@ -672,7 +662,7 @@ def CreateFcoeConfig(vlan_device_name, netcard) file_exists = SCR.Write(path(".target.string"), file_name, content) if file_exists - AddRevertCommand(Builtins.sformat("rm %1", file_name)) + AddRevertCommand(Builtins.sformat("/usr/bin/rm %1", file_name.shellescape)) # fill status map status_map = { "FCOE_ENABLE" => netcard["fcoe_enable"] || "yes", @@ -753,10 +743,10 @@ def DCBCapable(netcard) # 'lldpad' must be started to be able to use 'dcbtool' # -> is started in ServiceStatus() ( called in Read() before DetectNetworkCards() ) - command = Builtins.sformat("LANG=POSIX dcbtool gc %1 dcb", netcard) + command = Builtins.sformat("LANG=POSIX /usr/sbin/dcbtool gc %1 dcb", netcard.shellescape) Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map(SCR.Execute(path(".target.bash_output"), command)) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) status = "" @@ -1353,13 +1343,11 @@ def WriteCfgFiles if Ops.get_string(card, "dcb_required", "no") == "yes" # enable DCB on the interface command = Builtins.sformat( - "dcbtool sc %1 dcb on", - Ops.get_string(card, "dev_name", "") + "/usr/sbin/dcbtool sc %1 dcb on", + Ops.get_string(card, "dev_name", "").shellescape ) Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) != 0 # only warning, not necessarily an error @@ -1367,14 +1355,12 @@ def WriteCfgFiles end # enable App:FCoE on the interface command = Builtins.sformat( - "dcbtool sc %1 app:0 e:1 a:1 w:1", - Ops.get_string(card, "dev_name", "") + "/usr/sbin/dcbtool sc %1 app:0 e:1 a:1 w:1", + Ops.get_string(card, "dev_name", "").shellescape ) Builtins.y2milestone("Executing command: %1", command) - output = Convert.to_map( - SCR.Execute(path(".target.bash_output"), command) - ) + output = SCR.Execute(path(".target.bash_output"), command) Builtins.y2milestone("Output: %1", output) if Ops.get_integer(output, "exit", 255) != 0 # only warning, not necessarily an error From 652f18b8ef8bcf320b97819e4d8380226a9e6beb Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 20 Dec 2018 09:55:01 +0100 Subject: [PATCH 2/3] Changes --- package/yast2-fcoe-client.changes | 7 +++++++ package/yast2-fcoe-client.spec | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/package/yast2-fcoe-client.changes b/package/yast2-fcoe-client.changes index 7696f34..d4e5d6c 100644 --- a/package/yast2-fcoe-client.changes +++ b/package/yast2-fcoe-client.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Dec 20 08:54:28 UTC 2018 - Josef Reidinger + +- always use absolute path to binaries (bsc#1118291) +- properly escape shell arguments (bsc#1118291) +- 4.1.2 + ------------------------------------------------------------------- Sun Nov 25 14:04:13 UTC 2018 - Stasiek Michalski diff --git a/package/yast2-fcoe-client.spec b/package/yast2-fcoe-client.spec index 59f27fe..a365984 100644 --- a/package/yast2-fcoe-client.spec +++ b/package/yast2-fcoe-client.spec @@ -17,7 +17,7 @@ Name: yast2-fcoe-client -Version: 4.1.1 +Version: 4.1.2 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build From b0a31f9d4b5d3ae7c1e6b6b2361e4eeb976baa72 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 20 Dec 2018 10:04:34 +0100 Subject: [PATCH 3/3] fix test and also execute test during rpm build --- test/Makefile.am | 1 + test/fcoe_client_complex_include_test.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) mode change 100644 => 100755 test/fcoe_client_complex_include_test.rb diff --git a/test/Makefile.am b/test/Makefile.am index c1929cd..cb8d77d 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -1,5 +1,6 @@ TESTS = \ fcoe_client_write_spec.rb \ + fcoe_client_complex_include_test.rb \ DetectNetworkCards_spec.rb \ GetVlanInterfaces_spec.rb diff --git a/test/fcoe_client_complex_include_test.rb b/test/fcoe_client_complex_include_test.rb old mode 100644 new mode 100755 index c40c183..e2fc25f --- a/test/fcoe_client_complex_include_test.rb +++ b/test/fcoe_client_complex_include_test.rb @@ -2,6 +2,7 @@ require_relative "test_helper" Yast.import "Popup" +Yast.import "FcoeClient" class ComplexIncludeTest < Yast::Module def initialize @@ -34,7 +35,7 @@ def initialize it "smokes not" do expect(Yast::SCR) .to receive(:Execute) - .with(path(".target.bash_output"), "fipvlan -c -s eth9") + .with(path(".target.bash_output"), "/usr/sbin/fipvlan -c -s eth9") .and_return({"exit" => 0}) expect(Yast::FcoeClient).to receive(:GetFcoeVlanInterface).and_return("eth9.500") expect { subject.HandleInterfacesDialog(nil, event) }.to_not raise_error