From e9208369f3d4d727321d09269c982f17d4606d7a Mon Sep 17 00:00:00 2001 From: Ran Date: Tue, 7 Sep 2021 23:32:34 -0700 Subject: [PATCH] Throw exceptions when rule violating UDS paths been passed in. (#11663) Motivation: Currently, Netty is silently truncating all over the limit UDS paths and ignoring the `sun_path`'s null-termination role, which hurts compatibility with other UDS clients and servers. Modifications: Adding a validation in the JNI code, if the UDS path is not satisfying the system limit or Linux spec throw a NativeIoException. Result: All UDS paths Netty can successfully bind are connectable by other programs. --- .../netty/channel/epoll/LinuxSocketTest.java | 31 +++++++++++++++++++ .../src/main/c/netty_unix_socket.c | 12 ++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/LinuxSocketTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/LinuxSocketTest.java index ff85d3ac12c..9f16b2bfbfe 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/LinuxSocketTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/LinuxSocketTest.java @@ -15,6 +15,13 @@ */ package io.netty.channel.epoll; +import io.netty.channel.unix.DomainSocketAddress; +import io.netty.channel.unix.Errors.NativeIoException; +import io.netty.channel.unix.Socket; +import java.nio.charset.Charset; +import java.util.Random; +import java.util.UUID; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import java.io.IOException; @@ -65,4 +72,28 @@ public void execute() throws Throwable { socket.close(); } } + + @Test + public void testUnixDomainSocketTooLongPathFails() throws IOException { + // Most systems has a limit for UDS path of 108, 255 is generally too long. + StringBuilder socketPath = new StringBuilder("/tmp/"); + while (socketPath.length() < 255) { + socketPath.append(UUID.randomUUID()); + } + + final DomainSocketAddress domainSocketAddress = new DomainSocketAddress( + socketPath.toString()); + final Socket socket = Socket.newSocketDomain(); + try { + Exception exception = Assertions.assertThrows(NativeIoException.class, new Executable() { + @Override + public void execute() throws Throwable { + socket.bind(domainSocketAddress); + } + }); + Assertions.assertTrue(exception.getMessage().contains("too long")); + } finally { + socket.close(); + } + } } diff --git a/transport-native-unix-common/src/main/c/netty_unix_socket.c b/transport-native-unix-common/src/main/c/netty_unix_socket.c index 2524b412209..a6cd08f3086 100644 --- a/transport-native-unix-common/src/main/c/netty_unix_socket.c +++ b/transport-native-unix-common/src/main/c/netty_unix_socket.c @@ -764,12 +764,13 @@ static jint netty_unix_socket_bindDomainSocket(JNIEnv* env, jclass clazz, jint f jbyte* socket_path = (*env)->GetByteArrayElements(env, socketPath, 0); jint socket_path_len = (*env)->GetArrayLength(env, socketPath); - if (socket_path_len > sizeof(addr.sun_path)) { - socket_path_len = sizeof(addr.sun_path); + + if (socket_path_len > sizeof(addr.sun_path) || (socket_path_len == sizeof(addr.sun_path) && socket_path[socket_path_len] != '\0')) { + return -ENAMETOOLONG; } memcpy(addr.sun_path, socket_path, socket_path_len); - if (unlink((const char*) socket_path) == -1 && errno != ENOENT) { + if (unlink((const char*) addr.sun_path) == -1 && errno != ENOENT) { return -errno; } @@ -791,8 +792,9 @@ static jint netty_unix_socket_connectDomainSocket(JNIEnv* env, jclass clazz, jin jbyte* socket_path = (*env)->GetByteArrayElements(env, socketPath, 0); socket_path_len = (*env)->GetArrayLength(env, socketPath); - if (socket_path_len > sizeof(addr.sun_path)) { - socket_path_len = sizeof(addr.sun_path); + + if (socket_path_len > sizeof(addr.sun_path) || (socket_path_len == sizeof(addr.sun_path) && socket_path[socket_path_len] != '\0')) { + return -ENAMETOOLONG; } memcpy(addr.sun_path, socket_path, socket_path_len);