From e682967d7eb4bff978b011b03a6bf4b939745d1c Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 23 Aug 2020 13:47:52 +0200 Subject: LibCore: Prefer strlcpy over strncpy, fix overflow A malicious caller can create a SocketAddress for a local unix socket with an over-long name that does not fit into struct sock_addr_un. - Socket::connet: This caused the 'sun_path' field to overflow, probably overwriting the return pointer of the call frame, and thus crashing the process (in the best case). - SocketAddress::to_sockaddr_un: This triggered a RELEASE_ASSERT, and thus crashing the process. Both have been fixed to return a nice error code instead of crashing. --- Services/SystemServer/Service.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'Services/SystemServer/Service.cpp') diff --git a/Services/SystemServer/Service.cpp b/Services/SystemServer/Service.cpp index cb89033db2..f581c0c83d 100644 --- a/Services/SystemServer/Service.cpp +++ b/Services/SystemServer/Service.cpp @@ -135,7 +135,12 @@ void Service::setup_socket() } auto socket_address = Core::SocketAddress::local(m_socket_path); - auto un = socket_address.to_sockaddr_un(); + auto un_optional = socket_address.to_sockaddr_un(); + if (!un_optional.has_value()) { + dbg() << "Socket name " << m_socket_path << " is too long. BUG! This should have failed earlier!"; + ASSERT_NOT_REACHED(); + } + auto un = un_optional.value(); int rc = bind(m_socket_fd, (const sockaddr*)&un, sizeof(un)); if (rc < 0) { perror("bind"); @@ -358,6 +363,8 @@ Service::Service(const Core::ConfigFile& config, const StringView& name) ASSERT(!m_accept_socket_connections || (!m_socket_path.is_null() && m_lazy && m_multi_instance)); // MultiInstance doesn't work with KeepAlive. ASSERT(!m_multi_instance || !m_keep_alive); + // Socket path (plus NUL) must fit into the structs sent to the Kernel. + ASSERT(m_socket_path.length() < UNIX_PATH_MAX); if (!m_socket_path.is_null() && is_enabled()) { auto socket_permissions_string = config.read_entry(name, "SocketPermissions", "0600"); -- cgit v1.2.3