From: Razvan Becheriu Date: Wed, 28 Apr 2021 17:36:41 +0000 (+0300) Subject: [#1680] added check for execurtable permissions X-Git-Tag: Kea-1.9.8~173 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8f6d994d8de1bbf1cbb88c2c15aea86ed06cd1cb;p=thirdparty%2Fkea.git [#1680] added check for execurtable permissions --- diff --git a/src/hooks/dhcp/run_script/libloadtests/Makefile.am b/src/hooks/dhcp/run_script/libloadtests/Makefile.am index 40de572191..439045af15 100644 --- a/src/hooks/dhcp/run_script/libloadtests/Makefile.am +++ b/src/hooks/dhcp/run_script/libloadtests/Makefile.am @@ -4,6 +4,7 @@ AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += -I$(top_builddir)/src/hooks/dhcp/run_script -I$(top_srcdir)/src/hooks/dhcp/run_script AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CPPFLAGS += -DLIBRUN_SCRIPT_SO=\"$(abs_top_builddir)/src/hooks/dhcp/run_script/.libs/libdhcp_run_script.so\" +AM_CPPFLAGS += -DRUN_SCRIPT_TEST_SH=\"$(abs_top_builddir)/src/hooks/dhcp/run_script/tests/run_script_test.sh\" AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\" AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc b/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc index 1947c48b7f..b924b8ddf7 100644 --- a/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc +++ b/src/hooks/dhcp/run_script/libloadtests/load_unload_unittests.cc @@ -75,7 +75,7 @@ public: TEST_F(LibLoadTest, validLoad) { // Prepare parameters for the callout parameters library. ElementPtr params = Element::createMap(); - ElementPtr name = Element::create("test_script.sh"); + ElementPtr name = Element::create(RUN_SCRIPT_TEST_SH); params->set("name", name); ElementPtr sync = Element::create(false); params->set("sync", sync); @@ -104,12 +104,16 @@ TEST_F(LibLoadTest, invalidLoad) { params->set("name", name); EXPECT_FALSE(loadLibs()); - // Use valid name parameter type but use invalid sync parameter type. + // Use invalid name parameter. name = Element::create("script_name.sh"); params->set("name", name); + EXPECT_FALSE(loadLibs()); + + // Use valid name parameter type but use invalid sync parameter type. + name = Element::create(RUN_SCRIPT_TEST_SH); + params->set("name", name); ElementPtr sync = Element::create("data"); params->set("sync", sync); - EXPECT_FALSE(loadLibs()); } diff --git a/src/hooks/dhcp/run_script/run_script.cc b/src/hooks/dhcp/run_script/run_script.cc index a7748e95d8..d9a4eab588 100644 --- a/src/hooks/dhcp/run_script/run_script.cc +++ b/src/hooks/dhcp/run_script/run_script.cc @@ -32,6 +32,15 @@ RunScriptImpl::configure(LibraryHandle& handle) { if (name->getType() != Element::string) { isc_throw(InvalidParameter, "The 'name' parameter must be a string"); } + IOServicePtr io_service(new asiolink::IOService()); + try { + ProcessSpawn process(io_service, name->stringValue()); + if (!process.checkPermissions()) { + isc_throw(InvalidParameter, "The 'name' parameter must point to an executable"); + } + } catch (const isc::Exception& ex) { + isc_throw(InvalidParameter, "Invalid 'name' parameter: " << ex.what()); + } setName(name->stringValue()); ConstElementPtr sync = handle.getParameter("sync"); if (sync) { diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index 2f607182ec..a7ef6fe48f 100644 --- a/src/lib/asiolink/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include using namespace std; @@ -131,6 +132,12 @@ public: /// @param pid A process pid. void clearState(const pid_t pid); + /// @brief Check executable permissions. + /// + /// @return true if file has executable permissions, false otherwise. + /// @throw ProcessSpawnError if file does not exist. + bool checkPermissions() const; + private: /// @brief Copies the argument specified as a C++ string to the new @@ -357,6 +364,18 @@ ProcessSpawn::ProcessSpawn(IOServicePtr io_service, : impl_(new ProcessSpawnImpl(io_service, executable, args, vars)) { } +bool +ProcessSpawnImpl::checkPermissions() const { + struct stat st; + if (stat(executable_.c_str(), &st)) { + isc_throw(ProcessSpawnError, "File not found: " << executable_); + } + if (st.st_mode & S_IEXEC) { + return (true); + } + return (false); +} + std::string ProcessSpawn::getCommandLine() const { return (impl_->getCommandLine()); @@ -387,5 +406,10 @@ ProcessSpawn::clearState(const pid_t pid) { return (impl_->clearState(pid)); } +bool +ProcessSpawn::checkPermissions() const { + return (impl_->checkPermissions()); +} + } // namespace asiolink } // namespace isc diff --git a/src/lib/asiolink/process_spawn.h b/src/lib/asiolink/process_spawn.h index ca72ce8975..c810f50bd6 100644 --- a/src/lib/asiolink/process_spawn.h +++ b/src/lib/asiolink/process_spawn.h @@ -137,6 +137,12 @@ public: /// @param pid A process pid. void clearState(const pid_t pid); + /// @brief Check executable permissions. + /// + /// @return true if file has executable permissions, false otherwise. + /// @throw ProcessSpawnError if file does not exist. + bool checkPermissions() const; + private: /// @brief A smart pointer to the implementation of this class. diff --git a/src/lib/asiolink/tests/process_spawn_unittest.cc b/src/lib/asiolink/tests/process_spawn_unittest.cc index 11bb26d7d6..b1fe9c0b00 100644 --- a/src/lib/asiolink/tests/process_spawn_unittest.cc +++ b/src/lib/asiolink/tests/process_spawn_unittest.cc @@ -16,6 +16,8 @@ #include #include +#include + namespace { using namespace isc; @@ -343,4 +345,19 @@ TEST_F(ProcessSpawnTest, isRunning) { ASSERT_EQ(SIGCHLD, processed_signals_[0]); } +// This test verifies that the checkPermissions function throws if the file does +// not exist and returns true or false if the file is or it is not executable. +TEST_F(ProcessSpawnTest, checkPermissions) { + ProcessSpawn no_process(io_service_, "no-file"); + EXPECT_THROW_MSG(no_process.checkPermissions(), ProcessSpawnError, + "File not found: no-file"); + + std::string name = TEST_SCRIPT_SH; + name += ".in"; + ProcessSpawn invalid_process(io_service_, name); + ASSERT_FALSE(invalid_process.checkPermissions()); + ProcessSpawn process(io_service_, TEST_SCRIPT_SH); + ASSERT_TRUE(process.checkPermissions()); +} + } // end of anonymous namespace