From 2859e5000df617ed3edea778b4f9e228e006795e Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Tue, 13 Feb 2024 02:17:29 +0100 Subject: [PATCH] iox-#2185 Check for correct interface names --- iceoryx_posh/source/roudi/roudi.cpp | 10 +++++++- .../source/runtime/ipc_interface_base.cpp | 18 ++++++++++--- .../test/moduletests/test_posh_runtime.cpp | 4 ++- .../test_runtime_ipc_interface.cpp | 25 +++++++++++++------ 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/iceoryx_posh/source/roudi/roudi.cpp b/iceoryx_posh/source/roudi/roudi.cpp index 5960b6160a..3d837fda48 100644 --- a/iceoryx_posh/source/roudi/roudi.cpp +++ b/iceoryx_posh/source/roudi/roudi.cpp @@ -293,7 +293,15 @@ void RouDi::processMessage(const runtime::IpcMessage& message, { case runtime::IpcMessageType::REG: { - if (message.getNumberOfElements() != 6) + if (runtimeName.empty()) + { + IOX_LOG(ERROR, "Got message with empty runtime name!"); + } + else if (runtimeName.find(platform::IOX_PATH_SEPARATORS).has_value()) + { + IOX_LOG(ERROR, "Got message with a runtime name with invalid characters: \"" << runtimeName << "\"!"); + } + else if (message.getNumberOfElements() != 6) { IOX_LOG(ERROR, "Wrong number of parameters for \"IpcMessageType::REG\" from \"" << runtimeName << "\"received!"); diff --git a/iceoryx_posh/source/runtime/ipc_interface_base.cpp b/iceoryx_posh/source/runtime/ipc_interface_base.cpp index e27b929dab..59ff53e1d8 100644 --- a/iceoryx_posh/source/runtime/ipc_interface_base.cpp +++ b/iceoryx_posh/source/runtime/ipc_interface_base.cpp @@ -94,7 +94,18 @@ template IpcInterface::IpcInterface(const RuntimeName_t& runtimeName, const uint64_t maxMessages, const uint64_t messageSize) noexcept - : m_interfaceName( +{ + if (runtimeName.empty()) + { + IOX_PANIC("Then runtime name must not be empty"); + } + else if (runtimeName.find(iox::platform::IOX_PATH_SEPARATORS).has_value()) + { + IOX_LOG(FATAL, "The runtime name '" << runtimeName << "' contains path separators"); + IOX_PANIC("Invalid characters for runtime name"); + } + + m_interfaceName = ipcChannelNameToInterfaceName(runtimeName) .or_else([&runtimeName] { IOX_LOG(FATAL, @@ -103,9 +114,8 @@ IpcInterface::IpcInterface(const RuntimeName_t& runtimeName, << "' would exceed the maximum allowed size when used with the 'iox1_#_' prefix!"); IOX_PANIC("The runtime name exceeds the max size"); }) - .value()) - , m_runtimeName(runtimeName) -{ + .value(); + m_runtimeName = runtimeName; m_maxMessages = maxMessages; m_maxMessageSize = messageSize; if (m_maxMessageSize > platform::IoxIpcChannelType::MAX_MESSAGE_SIZE) diff --git a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp index 6ca838a509..579e03b772 100644 --- a/iceoryx_posh/test/moduletests/test_posh_runtime.cpp +++ b/iceoryx_posh/test/moduletests/test_posh_runtime.cpp @@ -128,7 +128,9 @@ TEST_F(PoshRuntime_test, ValidAppName) TEST_F(PoshRuntime_test, MaxAppNameLength) { ::testing::Test::RecordProperty("TEST_ID", "dfdf3ce1-c7d4-4c57-94ea-6ed9479371e3"); - std::string maxValidName(iox::MAX_RUNTIME_NAME_LENGTH, 's'); + RuntimeName_t dummy{"a"}; + auto prefixLength = runtime::ipcChannelNameToInterfaceName(dummy).value().size() - dummy.size(); + std::string maxValidName(iox::MAX_RUNTIME_NAME_LENGTH - prefixLength, 's'); auto& runtime = PoshRuntime::initRuntime(into>(maxValidName)); diff --git a/iceoryx_posh/test/moduletests/test_runtime_ipc_interface.cpp b/iceoryx_posh/test/moduletests/test_runtime_ipc_interface.cpp index 6c913144ac..224d47c285 100644 --- a/iceoryx_posh/test/moduletests/test_runtime_ipc_interface.cpp +++ b/iceoryx_posh/test/moduletests/test_runtime_ipc_interface.cpp @@ -20,8 +20,10 @@ #include "iox/message_queue.hpp" #include "iox/named_pipe.hpp" #include "iox/std_chrono_support.hpp" +#include "iox/std_string_support.hpp" #include "iox/unix_domain_socket.hpp" +#include "iceoryx_hoofs/testing/fatal_failure.hpp" #include "test.hpp" #include @@ -30,6 +32,7 @@ namespace { using namespace ::testing; using namespace iox; +using namespace iox::testing; using namespace iox::units::duration_literals; #if defined(__APPLE__) @@ -113,21 +116,27 @@ TYPED_TEST(IpcInterface_test, CreateWithTooLargeMessageSizeWillBeClampedToMaxMes EXPECT_TRUE(sut.isInitialized()); } -TYPED_TEST(IpcInterface_test, CreateNoNameLeadsToError) +TYPED_TEST(IpcInterface_test, CreateWithNoNameFails) { ::testing::Test::RecordProperty("TEST_ID", "3ffe2cf2-26f4-4b93-8baf-d997dc71e610"); - typename TestFixture::SutType sut(""); - EXPECT_FALSE(sut.openIpcChannel(PosixIpcChannelSide::SERVER)); - EXPECT_FALSE(sut.isInitialized()); + + IOX_EXPECT_FATAL_FAILURE([] { typename TestFixture::SutType sut(""); }, iox::er::FATAL); +} + +TYPED_TEST(IpcInterface_test, CreateWithTooLargeNameFails) +{ + ::testing::Test::RecordProperty("TEST_ID", "1463137c-ce3c-4a09-a568-f71ad10b558a"); + + auto tooLargeName = into>(std::string(iox::MAX_RUNTIME_NAME_LENGTH, 's')); + + IOX_EXPECT_FATAL_FAILURE([&] { typename TestFixture::SutType sut(tooLargeName); }, iox::er::FATAL); } -TYPED_TEST(IpcInterface_test, CreateWithLeadingSlashWorks) +TYPED_TEST(IpcInterface_test, CreateWithLeadingSlashFails) { ::testing::Test::RecordProperty("TEST_ID", "89340ebd-f80d-480b-833f-da37dff06cef"); - typename TestFixture::SutType sut(slashName); - EXPECT_TRUE(sut.openIpcChannel(PosixIpcChannelSide::SERVER)); - EXPECT_TRUE(sut.isInitialized()); + IOX_EXPECT_FATAL_FAILURE([] { typename TestFixture::SutType sut(slashName); }, iox::er::FATAL); } TYPED_TEST(IpcInterface_test, CreateAgainWorks)