diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index 470bde5145..4ed19cd937 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -39,7 +39,7 @@ int socket_address_listen( assert(a); - r = socket_address_verify(a); + r = socket_address_verify(a, true); if (r < 0) return r; diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index d94bac4287..9bae1552fc 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -15,6 +15,7 @@ #include #include "alloc-util.h" +#include "escape.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" @@ -111,7 +112,7 @@ int socket_address_parse(SocketAddress *a, const char *s) { size_t l; l = strlen(s+1); - if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminate sockets here + if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminated sockets here * when parsing, even though abstract namespace sockets * explicitly allow embedded NUL bytes and don't consider * them special. But it's simply annoying to debug such @@ -261,9 +262,12 @@ int socket_address_parse_netlink(SocketAddress *a, const char *s) { return 0; } -int socket_address_verify(const SocketAddress *a) { +int socket_address_verify(const SocketAddress *a, bool strict) { assert(a); + /* With 'strict' we enforce additional sanity constraints which are not set by the standard, + * but should only apply to sockets we create ourselves. */ + switch (socket_address_family(a)) { case AF_INET: @@ -293,19 +297,20 @@ int socket_address_verify(const SocketAddress *a) { case AF_UNIX: if (a->size < offsetof(struct sockaddr_un, sun_path)) return -EINVAL; - if (a->size > sizeof(struct sockaddr_un)+1) /* Allow one extra byte, since getsockname() on Linux will - * append a NUL byte if we have path sockets that are above - * sun_path' full size */ + if (a->size > sizeof(struct sockaddr_un) + !strict) + /* If !strict, allow one extra byte, since getsockname() on Linux will append + * a NUL byte if we have path sockets that are above sun_path's full size. */ return -EINVAL; if (a->size > offsetof(struct sockaddr_un, sun_path) && - a->sockaddr.un.sun_path[0] != 0) { /* Only validate file system sockets here */ - + a->sockaddr.un.sun_path[0] != 0 && + strict) { + /* Only validate file system sockets here, and only in strict mode */ const char *e; e = memchr(a->sockaddr.un.sun_path, 0, sizeof(a->sockaddr.un.sun_path)); if (e) { - /* If there's an embedded NUL byte, make sure the size of the socket addresses matches it */ + /* If there's an embedded NUL byte, make sure the size of the socket address matches it */ if (a->size != offsetof(struct sockaddr_un, sun_path) + (e - a->sockaddr.un.sun_path) + 1) return -EINVAL; } else { @@ -353,7 +358,10 @@ int socket_address_print(const SocketAddress *a, char **ret) { assert(a); assert(ret); - r = socket_address_verify(a); + r = socket_address_verify(a, false); /* We do non-strict validation, because we want to be + * able to pretty-print any socket the kernel considers + * valid. We still need to do validation to know if we + * can meaningfully print the address. */ if (r < 0) return r; @@ -386,8 +394,8 @@ bool socket_address_equal(const SocketAddress *a, const SocketAddress *b) { assert(b); /* Invalid addresses are unequal to all */ - if (socket_address_verify(a) < 0 || - socket_address_verify(b) < 0) + if (socket_address_verify(a, false) < 0 || + socket_address_verify(b, false) < 0) return false; if (a->type != b->type) @@ -642,31 +650,39 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ } case AF_UNIX: - if (salen <= offsetof(struct sockaddr_un, sun_path)) { + if (salen <= offsetof(struct sockaddr_un, sun_path) || + (sa->un.sun_path[0] == 0 && salen == offsetof(struct sockaddr_un, sun_path) + 1)) { + /* The name must have at least one character (and the leading NUL does not count) */ + p = strdup(""); if (!p) return -ENOMEM; - } else if (sa->un.sun_path[0] == 0) { - /* abstract */ - - /* FIXME: We assume we can print the - * socket path here and that it hasn't - * more than one NUL byte. That is - * actually an invalid assumption */ - - p = new(char, sizeof(sa->un.sun_path)+1); - if (!p) - return -ENOMEM; - - p[0] = '@'; - memcpy(p+1, sa->un.sun_path+1, sizeof(sa->un.sun_path)-1); - p[sizeof(sa->un.sun_path)] = 0; - } else { - p = strndup(sa->un.sun_path, sizeof(sa->un.sun_path)); + size_t path_len = salen - offsetof(struct sockaddr_un, sun_path); + + if (sa->un.sun_path[0] == 0) { + /* Abstract socket. When parsing address information from, we + * explicitly reject overly long paths and paths with embedded NULs. + * But we might get such a socket from the outside. Let's return + * something meaningful and printable in this case. */ + + _cleanup_free_ char *e = NULL; + + e = cescape_length(sa->un.sun_path + 1, path_len - 1); + if (!e) + return -ENOMEM; + + p = strjoin("@", e); + } else { + if (sa->un.sun_path[path_len - 1] == '\0') + /* We expect a terminating NUL and don't print it */ + path_len --; + + p = cescape_length(sa->un.sun_path, path_len); + } if (!p) - return -ENOMEM; + return -ENOMEM; } break; diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 8090e21657..37b1bca81a 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -70,7 +70,7 @@ int socket_address_parse(SocketAddress *a, const char *s); int socket_address_parse_and_warn(SocketAddress *a, const char *s); int socket_address_parse_netlink(SocketAddress *a, const char *s); int socket_address_print(const SocketAddress *a, char **p); -int socket_address_verify(const SocketAddress *a) _pure_; +int socket_address_verify(const SocketAddress *a, bool strict) _pure_; int sockaddr_un_unlink(const struct sockaddr_un *sa); diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 687caf83be..349d866442 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "async.h" +#include "escape.h" #include "exit-status.h" #include "fd-util.h" #include "fileio.h" @@ -62,6 +63,9 @@ static void test_socket_address_parse_one(const char *in, int ret, int family, c } } +#define SUN_PATH_LEN (sizeof(((struct sockaddr_un){}).sun_path)) +assert_cc(sizeof(((struct sockaddr_un){}).sun_path) == 108); + static void test_socket_address_parse(void) { log_info("/* %s */", __func__); @@ -95,8 +99,21 @@ static void test_socket_address_parse(void) { test_socket_address_parse_one("[::1]:8888", 0, AF_INET6, NULL); test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET, NULL); test_socket_address_parse_one("/foo/bar", 0, AF_UNIX, NULL); + test_socket_address_parse_one("/", 0, AF_UNIX, NULL); test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); + { + char aaa[SUN_PATH_LEN + 1] = "@"; + + memset(aaa + 1, 'a', SUN_PATH_LEN - 1); + char_array_0(aaa); + + test_socket_address_parse_one(aaa, -EINVAL, 0, NULL); + + aaa[SUN_PATH_LEN - 1] = '\0'; + test_socket_address_parse_one(aaa, 0, AF_UNIX, NULL); + } + test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, NULL); test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); @@ -104,6 +121,40 @@ static void test_socket_address_parse(void) { test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); } +static void test_socket_print_unix_one(const char *in, size_t len_in, const char *expected) { + _cleanup_free_ char *out = NULL, *c = NULL; + + SocketAddress a = { .sockaddr = { .un = { .sun_family = AF_UNIX } }, + .size = offsetof(struct sockaddr_un, sun_path) + len_in, + .type = SOCK_STREAM, + }; + memcpy(a.sockaddr.un.sun_path, in, len_in); + + assert_se(socket_address_print(&a, &out) >= 0); + assert_se(c = cescape(in)); + log_info("\"%s\" → \"%s\" (expect \"%s\")", in, out, expected); + assert_se(streq(out, expected)); +} + +static void test_socket_print_unix(void) { + log_info("/* %s */", __func__); + + /* Some additional tests for abstract addresses which we don't parse */ + + test_socket_print_unix_one("\0\0\0\0", 4, "@\\000\\000\\000"); + test_socket_print_unix_one("@abs", 5, "@abs"); + test_socket_print_unix_one("\n", 2, "\\n"); + test_socket_print_unix_one("", 1, ""); + test_socket_print_unix_one("\0", 1, ""); + test_socket_print_unix_one("\0_________________________there's 108 characters in this string_____________________________________________", 108, + "@_________________________there\\'s 108 characters in this string_____________________________________________"); + test_socket_print_unix_one("////////////////////////////////////////////////////////////////////////////////////////////////////////////", 108, + "////////////////////////////////////////////////////////////////////////////////////////////////////////////"); + test_socket_print_unix_one("////////////////////////////////////////////////////////////////////////////////////////////////////////////", 109, + "////////////////////////////////////////////////////////////////////////////////////////////////////////////"); + test_socket_print_unix_one("\0\a\b\n\255", 6, "@\\a\\b\\n\\255\\000"); +} + static void test_socket_address_parse_netlink(void) { SocketAddress a; @@ -748,6 +799,7 @@ int main(int argc, char *argv[]) { test_ifname_valid(); test_socket_address_parse(); + test_socket_print_unix(); test_socket_address_parse_netlink(); test_socket_address_equal(); test_socket_address_get_path();