diff options
author | Ben Wiederhake <BenWiederhake.GitHub@gmx.de> | 2020-08-23 13:47:52 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2020-08-24 00:45:03 +0200 |
commit | e682967d7eb4bff978b011b03a6bf4b939745d1c (patch) | |
tree | c145d8258ad83979a0385a455fad69521f57402a /Libraries/LibCore | |
parent | d419a780aed4a8111ab30531797d0e1afe6f02c4 (diff) | |
download | serenity-e682967d7eb4bff978b011b03a6bf4b939745d1c.zip |
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.
Diffstat (limited to 'Libraries/LibCore')
-rw-r--r-- | Libraries/LibCore/LocalServer.cpp | 7 | ||||
-rw-r--r-- | Libraries/LibCore/Socket.cpp | 6 | ||||
-rw-r--r-- | Libraries/LibCore/SocketAddress.h | 8 |
3 files changed, 17 insertions, 4 deletions
diff --git a/Libraries/LibCore/LocalServer.cpp b/Libraries/LibCore/LocalServer.cpp index ceadadfdb3..8b41ddcf21 100644 --- a/Libraries/LibCore/LocalServer.cpp +++ b/Libraries/LibCore/LocalServer.cpp @@ -121,7 +121,12 @@ bool LocalServer::listen(const String& address) #endif auto socket_address = SocketAddress::local(address); - auto un = socket_address.to_sockaddr_un(); + auto un_optional = socket_address.to_sockaddr_un(); + if (!un_optional.has_value()) { + perror("bind"); + return false; + } + auto un = un_optional.value(); rc = ::bind(m_fd, (const sockaddr*)&un, sizeof(un)); if (rc < 0) { perror("bind"); diff --git a/Libraries/LibCore/Socket.cpp b/Libraries/LibCore/Socket.cpp index a12c57993a..6685839a27 100644 --- a/Libraries/LibCore/Socket.cpp +++ b/Libraries/LibCore/Socket.cpp @@ -111,6 +111,12 @@ bool Socket::connect(const SocketAddress& address) sockaddr_un saddr; saddr.sun_family = AF_LOCAL; + auto dest_address = address.to_string(); + if (dest_address.length() >= sizeof(saddr.sun_path)) { + fprintf(stderr, "Core::Socket: Failed to connect() to %s: Path is too long!\n", dest_address.characters()); + errno = EINVAL; + return false; + } strcpy(saddr.sun_path, address.to_string().characters()); m_destination_address = address; diff --git a/Libraries/LibCore/SocketAddress.h b/Libraries/LibCore/SocketAddress.h index 621687fe13..1ffa3da24f 100644 --- a/Libraries/LibCore/SocketAddress.h +++ b/Libraries/LibCore/SocketAddress.h @@ -43,7 +43,7 @@ public: Local }; - SocketAddress() {} + SocketAddress() { } SocketAddress(const IPv4Address& address) : m_type(Type::IPv4) , m_ipv4_address(address) @@ -82,12 +82,14 @@ public: } } - sockaddr_un to_sockaddr_un() const + Optional<sockaddr_un> to_sockaddr_un() const { ASSERT(type() == Type::Local); sockaddr_un address; address.sun_family = AF_LOCAL; - RELEASE_ASSERT(m_local_address.length() < (int)sizeof(address.sun_path)); + if (m_local_address.length() >= sizeof(address.sun_path)) { + return {}; + } strcpy(address.sun_path, m_local_address.characters()); return address; } |