diff --git a/cfgmgr/intfmgr.cpp b/cfgmgr/intfmgr.cpp index e7365f538d53..18f1d8e2f1ae 100644 --- a/cfgmgr/intfmgr.cpp +++ b/cfgmgr/intfmgr.cpp @@ -41,14 +41,14 @@ void IntfMgr::setIntfIp(const string &alias, const string &opCmd, if (ipPrefix.isV4()) { (prefixLen < 31) ? - (cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " broadcast " << broadcastIpStr <<" dev " << alias) : - (cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " dev " << alias); + (cmd << IP_CMD << " address " << shellquote(opCmd) << " " << shellquote(ipPrefixStr) << " broadcast " << shellquote(broadcastIpStr) <<" dev " << shellquote(alias)) : + (cmd << IP_CMD << " address " << shellquote(opCmd) << " " << shellquote(ipPrefixStr) << " dev " << shellquote(alias)); } else { (prefixLen < 127) ? - (cmd << IP_CMD << " -6 address " << opCmd << " " << ipPrefixStr << " broadcast " << broadcastIpStr << " dev " << alias) : - (cmd << IP_CMD << " -6 address " << opCmd << " " << ipPrefixStr << " dev " << alias); + (cmd << IP_CMD << " -6 address " << shellquote(opCmd) << " " << shellquote(ipPrefixStr) << " broadcast " << shellquote(broadcastIpStr) << " dev " << shellquote(alias)) : + (cmd << IP_CMD << " -6 address " << shellquote(opCmd) << " " << shellquote(ipPrefixStr) << " dev " << shellquote(alias)); } int ret = swss::exec(cmd.str(), res); @@ -65,11 +65,11 @@ void IntfMgr::setIntfVrf(const string &alias, const string vrfName) if (!vrfName.empty()) { - cmd << IP_CMD << " link set " << alias << " master " << vrfName; + cmd << IP_CMD << " link set " << shellquote(alias) << " master " << shellquote(vrfName); } else { - cmd << IP_CMD << " link set " << alias << " nomaster"; + cmd << IP_CMD << " link set " << shellquote(alias) << " nomaster"; } EXEC_WITH_ERROR_THROW(cmd.str(), res); } diff --git a/cfgmgr/portmgr.cpp b/cfgmgr/portmgr.cpp index 03b2e61eb69e..765198833a5d 100644 --- a/cfgmgr/portmgr.cpp +++ b/cfgmgr/portmgr.cpp @@ -25,7 +25,7 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu) string res; // ip link set dev mtu - cmd << IP_CMD << " link set dev " << alias << " mtu " << mtu; + cmd << IP_CMD << " link set dev " << shellquote(alias) << " mtu " << shellquote(mtu); EXEC_WITH_ERROR_THROW(cmd.str(), res); // Set the port MTU in application database to update both @@ -44,7 +44,7 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up) string res; // ip link set dev [up|down] - cmd << IP_CMD << " link set dev " << alias << (up ? " up" : " down"); + cmd << IP_CMD << " link set dev " << shellquote(alias) << (up ? " up" : " down"); EXEC_WITH_ERROR_THROW(cmd.str(), res); vector fvs; diff --git a/cfgmgr/shellcmd.h b/cfgmgr/shellcmd.h index 7507da289244..a8a7afa51c5e 100644 --- a/cfgmgr/shellcmd.h +++ b/cfgmgr/shellcmd.h @@ -1,6 +1,9 @@ #ifndef __SHELLCMD__ #define __SHELLCMD__ +#include +#include + #define IP_CMD "/sbin/ip" #define BRIDGE_CMD "/sbin/bridge" #define BRCTL_CMD "/sbin/brctl" @@ -18,4 +21,10 @@ } \ }) +static inline std::string shellquote(const std::string& str) +{ + static const std::regex re("([$`\"\\\n])"); + return "\"" + std::regex_replace(str, re, "\\$1") + "\""; +} + #endif /* __SHELLCMD__ */ diff --git a/cfgmgr/teammgr.cpp b/cfgmgr/teammgr.cpp index 448a7ac8007d..2149e1ce5ff1 100644 --- a/cfgmgr/teammgr.cpp +++ b/cfgmgr/teammgr.cpp @@ -319,7 +319,7 @@ bool TeamMgr::setLagAdminStatus(const string &alias, const string &admin_status) string res; // ip link set dev [up|down] - cmd << IP_CMD << " link set dev " << alias << " " << admin_status; + cmd << IP_CMD << " link set dev " << shellquote(alias) << " " << shellquote(admin_status); EXEC_WITH_ERROR_THROW(cmd.str(), res); SWSS_LOG_NOTICE("Set port channel %s admin status to %s", @@ -336,7 +336,7 @@ bool TeamMgr::setLagMtu(const string &alias, const string &mtu) string res; // ip link set dev mtu - cmd << IP_CMD << " link set dev " << alias << " mtu " << mtu; + cmd << IP_CMD << " link set dev " << shellquote(alias) << " mtu " << shellquote(mtu); EXEC_WITH_ERROR_THROW(cmd.str(), res); vector fvs; @@ -423,7 +423,7 @@ bool TeamMgr::removeLag(const string &alias) stringstream cmd; string res; - cmd << TEAMD_CMD << " -k -t " << alias; + cmd << TEAMD_CMD << " -k -t " << shellquote(alias); EXEC_WITH_ERROR_THROW(cmd.str(), res); SWSS_LOG_NOTICE("Stop port channel %s", alias.c_str()); @@ -451,8 +451,8 @@ task_process_status TeamMgr::addLagMember(const string &lag, const string &membe // Set admin down LAG member (required by teamd) and enslave it // ip link set dev down; // teamdctl port add ; - cmd << IP_CMD << " link set dev " << member << " down; "; - cmd << TEAMDCTL_CMD << " " << lag << " port add " << member; + cmd << IP_CMD << " link set dev " << shellquote(member) << " down; "; + cmd << TEAMDCTL_CMD << " " << shellquote(lag) << " port add " << shellquote(member); if (exec(cmd.str(), res) != 0) { @@ -505,7 +505,7 @@ task_process_status TeamMgr::addLagMember(const string &lag, const string &membe // ip link set dev [up|down] cmd.str(string()); - cmd << IP_CMD << " link set dev " << member << " " << admin_status; + cmd << IP_CMD << " link set dev " << shellquote(member) << " " << shellquote(admin_status); EXEC_WITH_ERROR_THROW(cmd.str(), res); fvs.clear(); @@ -550,8 +550,8 @@ bool TeamMgr::removeLagMember(const string &lag, const string &member) // ip link set dev [up|down]; // ip link set dev mtu - cmd << IP_CMD << " link set dev " << member << " " << admin_status << "; "; - cmd << IP_CMD << " link set dev " << member << " mtu " << mtu; + cmd << IP_CMD << " link set dev " << shellquote(member) << " " << shellquote(admin_status) << "; "; + cmd << IP_CMD << " link set dev " << shellquote(member) << " mtu " << shellquote(mtu); EXEC_WITH_ERROR_THROW(cmd.str(), res); fvs.clear(); diff --git a/cfgmgr/vlanmgr.cpp b/cfgmgr/vlanmgr.cpp index fc5949252e20..d8834d12d929 100644 --- a/cfgmgr/vlanmgr.cpp +++ b/cfgmgr/vlanmgr.cpp @@ -134,11 +134,11 @@ bool VlanMgr::setHostVlanAdminState(int vlan_id, const string &admin_status) // The command should be generated as: // /sbin/ip link set Vlan{{vlan_id}} {{admin_status}} - const std::string cmds = std::string("") - + IP_CMD + " link set " + VLAN_PREFIX + std::to_string(vlan_id) + " " + admin_status; + ostringstream cmds; + cmds << IP_CMD " link set " VLAN_PREFIX + std::to_string(vlan_id) + " " << shellquote(admin_status); std::string res; - EXEC_WITH_ERROR_THROW(cmds, res); + EXEC_WITH_ERROR_THROW(cmds.str(), res); return true; } @@ -177,14 +177,14 @@ bool VlanMgr::addHostVlanMember(int vlan_id, const string &port_alias, const str // /bin/bash -c "/sbin/ip link set {{port_alias}} master Bridge && // /sbin/bridge vlan del vid 1 dev {{ port_alias }} && // /sbin/bridge vlan add vid {{vlan_id}} dev {{port_alias}} {{tagging_mode}}" - const std::string cmds = std::string("") - + BASH_CMD + " -c \"" - + IP_CMD + " link set " + port_alias + " master " + DOT1Q_BRIDGE_NAME + " && " - + BRIDGE_CMD + " vlan del vid " + DEFAULT_VLAN_ID + " dev " + port_alias + " && " - + BRIDGE_CMD + " vlan add vid " + std::to_string(vlan_id) + " dev " + port_alias + " " + tagging_cmd + "\""; + ostringstream cmds, inner; + inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " + BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " + BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; + cmds << BASH_CMD " -c " << shellquote(inner.str()); std::string res; - EXEC_WITH_ERROR_THROW(cmds, res); + EXEC_WITH_ERROR_THROW(cmds.str(), res); return true; } @@ -202,17 +202,17 @@ bool VlanMgr::removeHostVlanMember(int vlan_id, const string &port_alias) // else exit $ret; fi )' // When port is not member of any VLAN, it shall be detached from Dot1Q bridge! - const std::string cmds = std::string("") - + BASH_CMD + " -c \'" - + BRIDGE_CMD + " vlan del vid " + std::to_string(vlan_id) + " dev " + port_alias + " && ( " - + BRIDGE_CMD + " vlan show dev " + port_alias + " | " - + GREP_CMD + " -q None; ret=$?; if [ $ret -eq 0 ]; then " - + IP_CMD + " link set " + port_alias + " nomaster; " - + "elif [ $ret -eq 1 ]; then exit 0; " - + "else exit $ret; fi )\'"; + ostringstream cmds, inner; + inner << BRIDGE_CMD " vlan del vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " && ( " + BRIDGE_CMD " vlan show dev " << shellquote(port_alias) << " | " + GREP_CMD " -q None; ret=$?; if [ $ret -eq 0 ]; then " + IP_CMD " link set " << shellquote(port_alias) << " nomaster; " + "elif [ $ret -eq 1 ]; then exit 0; " + "else exit $ret; fi )"; + cmds << BASH_CMD " -c " << shellquote(cmds.str()); std::string res; - EXEC_WITH_ERROR_THROW(cmds, res); + EXEC_WITH_ERROR_THROW(cmds.str(), res); return true; } diff --git a/cfgmgr/vrfmgr.cpp b/cfgmgr/vrfmgr.cpp index c8edf2151222..d8048a6b3513 100644 --- a/cfgmgr/vrfmgr.cpp +++ b/cfgmgr/vrfmgr.cpp @@ -99,7 +99,7 @@ bool VrfMgr::delLink(const string& vrfName) return false; } - cmd << IP_CMD << " link del " << vrfName; + cmd << IP_CMD << " link del " << shellquote(vrfName); EXEC_WITH_ERROR_THROW(cmd.str(), res); recycleTable(m_vrfTableMap[vrfName]); @@ -126,14 +126,14 @@ bool VrfMgr::setLink(const string& vrfName) return false; } - cmd << IP_CMD << " link add " << vrfName << " type vrf table " << table; + cmd << IP_CMD << " link add " << shellquote(vrfName) << " type vrf table " << table; EXEC_WITH_ERROR_THROW(cmd.str(), res); m_vrfTableMap.emplace(vrfName, table); cmd.str(""); cmd.clear(); - cmd << IP_CMD << " link set " << vrfName << " up"; + cmd << IP_CMD << " link set " << shellquote(vrfName) << " up"; EXEC_WITH_ERROR_THROW(cmd.str(), res); return true; diff --git a/cfgmgr/vxlanmgr.cpp b/cfgmgr/vxlanmgr.cpp index 40d3362406b3..6706407bdb26 100644 --- a/cfgmgr/vxlanmgr.cpp +++ b/cfgmgr/vxlanmgr.cpp @@ -40,111 +40,113 @@ static std::string getVxlanIfName(const swss::VxlanMgr::VxlanInfo & info) // Commands #define RET_SUCCESS 0 -#define EXECUTE(CMD, RESULT) swss::exec(std::string() + BASH_CMD + " -c \"" + CMD + "\"", RESULT); static int cmdCreateVxlan(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link add {{VXLAN}} type vxlan id {{VNI}} [local {{SOURCE IP}}] dstport 4789 - const std::string cmd = std::string("") - + IP_CMD " link add " - + info.m_vxlan - + " type vxlan id " - + info.m_vni - + " " - + (info.m_sourceIp.empty() ? "" : (" local " + info.m_sourceIp)) - + " dstport 4789"; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link add " + << shellquote(info.m_vxlan) + << " type vxlan id " + << shellquote(info.m_vni) + << " "; + if (!info.m_sourceIp.empty()) + { + cmd << " local " << shellquote(info.m_sourceIp); + } + cmd << " dstport 4789"; + return swss::exec(cmd.str(), res); } static int cmdUpVxlan(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link set dev {{VXLAN}} up - const std::string cmd = std::string("") - + IP_CMD " link set dev " - + info.m_vxlan - + " up"; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link set dev " + << shellquote(info.m_vxlan) + << " up"; + return swss::exec(cmd.str(), res); } static int cmdCreateVxlanIf(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link add {{VXLAN_IF}} type bridge - const std::string cmd = std::string("") - + IP_CMD " link add " - + info.m_vxlanIf - + " type bridge"; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link add " + << shellquote(info.m_vxlanIf) + << " type bridge"; + return swss::exec(cmd.str(), res); } static int cmdAddVxlanIntoVxlanIf(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // brctl addif {{VXLAN_IF}} {{VXLAN}} - const std::string cmd = std::string("") - + BRCTL_CMD " addif " - + info.m_vxlanIf - + " " - + info.m_vxlan; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << BRCTL_CMD " addif " + << shellquote(info.m_vxlanIf) + << " " + << shellquote(info.m_vxlan); + return swss::exec(cmd.str(), res); } static int cmdAttachVxlanIfToVnet(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link set dev {{VXLAN_IF}} master {{VNET}} - const std::string cmd = std::string("") - + IP_CMD " link set dev " - + info.m_vxlanIf - + " master " - + info.m_vnet; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link set dev " + << shellquote(info.m_vxlanIf) + << " master " + << shellquote(info.m_vnet); + return swss::exec(cmd.str(), res); } static int cmdUpVxlanIf(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link set dev {{VXLAN_IF}} up - const std::string cmd = std::string("") - + IP_CMD " link set dev " - + info.m_vxlanIf - + " up"; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link set dev " + << shellquote(info.m_vxlanIf) + << " up"; + return swss::exec(cmd.str(), res); } static int cmdDeleteVxlan(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link del dev {{VXLAN}} - const std::string cmd = std::string("") - + IP_CMD " link del dev " - + info.m_vxlan; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link del dev " + << shellquote(info.m_vxlan); + return swss::exec(cmd.str(), res); } static int cmdDeleteVxlanFromVxlanIf(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // brctl delif {{VXLAN_IF}} {{VXLAN}} - const std::string cmd = std::string("") - + BRCTL_CMD " delif " - + info.m_vxlanIf - + " " - + info.m_vxlan; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << BRCTL_CMD " delif " + << shellquote(info.m_vxlanIf) + << " " + << shellquote(info.m_vxlan); + return swss::exec(cmd.str(), res); } static int cmdDeleteVxlanIf(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link del {{VXLAN_IF}} - const std::string cmd = std::string("") - + IP_CMD " link del " - + info.m_vxlanIf; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link del " + << shellquote(info.m_vxlanIf); + return swss::exec(cmd.str(), res); } static int cmdDetachVxlanIfFromVnet(const swss::VxlanMgr::VxlanInfo & info, std::string & res) { // ip link set dev {{VXLAN_IF}} nomaster - const std::string cmd = std::string("") - + IP_CMD " link set dev " - + info.m_vxlanIf - + " nomaster"; - return EXECUTE(cmd, res); + ostringstream cmd; + cmd << IP_CMD " link set dev " + << shellquote(info.m_vxlanIf) + << " nomaster"; + return swss::exec(cmd.str(), res); } // Vxlanmgr @@ -540,7 +542,7 @@ void VxlanMgr::clearAllVxlanDevices() { std::string stdout; const std::string cmd = std::string("") + IP_CMD + " link"; - int ret = EXECUTE(cmd, stdout); + int ret = swss::exec(cmd, stdout); if (ret != 0) { SWSS_LOG_ERROR("Cannot get devices by command : %s", cmd.c_str()); @@ -570,4 +572,3 @@ void VxlanMgr::clearAllVxlanDevices() } } } - diff --git a/configure.ac b/configure.ac index 3d22f0d02b6a..44071996d5b7 100644 --- a/configure.ac +++ b/configure.ac @@ -34,7 +34,7 @@ AC_ARG_ENABLE(debug, esac],[debug=false]) AM_CONDITIONAL(DEBUG, test x$debug = xtrue) -CFLAGS_COMMON="-std=c++11 -Wall -fPIC -Wno-write-strings -I/usr/include/libnl3 -I/usr/include/swss" +CFLAGS_COMMON="-std=c++14 -Wall -fPIC -Wno-write-strings -I/usr/include/libnl3 -I/usr/include/swss" AM_CONDITIONAL(sonic_asic_platform_barefoot, test x$CONFIGURED_PLATFORM = xbarefoot) AM_COND_IF([sonic_asic_platform_barefoot], diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index 20d41b7b8d93..bf39f8f0012c 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -18,6 +18,8 @@ #include #include +#include +#include using namespace std; using namespace swss; @@ -58,11 +60,12 @@ LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) : * This piece of information is used by SNMP. */ if (!key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX)) { - string cmd, res; - cmd = "cat /sys/class/net/" + key + "/operstate"; + ostringstream cmd; + string res; + cmd << "cat /sys/class/net/" << shellquote(key) << "/operstate"; try { - EXEC_WITH_ERROR_THROW(cmd, res); + EXEC_WITH_ERROR_THROW(cmd.str(), res); } catch (...) { @@ -132,13 +135,14 @@ LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) : m_ifindexOldNameMap[idx_p->if_index] = key; - string cmd, res; + ostringstream cmd; + string res; /* Bring down the existing kernel interfaces */ SWSS_LOG_INFO("Bring down old interface %s(%d)", key.c_str(), idx_p->if_index); - cmd = "ip link set " + key + " down"; + cmd << "ip link set " << quoted(key) << " down"; try { - swss::exec(cmd, res); + swss::exec(cmd.str(), res); } catch (...) { diff --git a/tests/Makefile.am b/tests/Makefile.am index 8818cbf57f86..b6bff493a6b2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -13,7 +13,8 @@ endif CFLAGS_GTEST = LDADD_GTEST = -L/usr/src/gtest -tests_SOURCES = swssnet_ut.cpp request_parser_ut.cpp ../orchagent/request_parser.cpp +tests_SOURCES = swssnet_ut.cpp request_parser_ut.cpp ../orchagent/request_parser.cpp \ + quoted_ut.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I../orchagent diff --git a/tests/quoted_ut.cpp b/tests/quoted_ut.cpp new file mode 100644 index 000000000000..cd8fef409f22 --- /dev/null +++ b/tests/quoted_ut.cpp @@ -0,0 +1,40 @@ +#include +#include +#include +#include +#include "swssnet.h" +#include "cfgmgr/shellcmd.h" + +using namespace std; +using namespace swss; + +#define DOT1Q_BRIDGE_NAME "Bridge" +#define DEFAULT_VLAN_ID "1" + +TEST(quoted, copy1_v6) +{ + ostringstream cmd; + string key = "Ethernet0"; + cmd << "ip link set " << shellquote(key) << " down"; + EXPECT_EQ(cmd.str(), "ip link set \"Ethernet0\" down"); + + ostringstream cmd2; + key = "; rm -rf /; echo '\"'"; + cmd2 << "ip link set " << shellquote(key) << " down"; + EXPECT_EQ(cmd2.str(), "ip link set \"; rm -rf /; echo '\\\"'\" down"); + + ostringstream cmds, inner; + string port_alias = "etp1"; + string tagging_cmd = "pvid untagged"; + int vlan_id = 111; + inner << IP_CMD " link set " << shellquote(port_alias) << " master " DOT1Q_BRIDGE_NAME " && " + BRIDGE_CMD " vlan del vid " DEFAULT_VLAN_ID " dev " << shellquote(port_alias) << " && " + BRIDGE_CMD " vlan add vid " + std::to_string(vlan_id) + " dev " << shellquote(port_alias) << " " + tagging_cmd; + cmds << BASH_CMD " -c " << shellquote(inner.str()); + EXPECT_EQ(cmds.str(), "/bin/bash -c \"/sbin/ip link set \\\"etp1\\\" master Bridge && /sbin/bridge vlan del vid 1 dev \\\"etp1\\\" && /sbin/bridge vlan add vid 111 dev \\\"etp1\\\" pvid untagged\""); + + ostringstream cmd4; + key = "$(echo hi)"; + cmd4 << "cat /sys/class/net/" << shellquote(key) << "/operstate"; + EXPECT_EQ(cmd4.str(), "cat /sys/class/net/\"\\$(echo hi)\"/operstate"); +}