On Sun, Nov 26, 2023 at 07:12:47PM -0800, Dev Email wrote:
> >Sypnosis: getsockname() reports more information in the socket address
> buffer than what actually exists
>
> >Category: library
>
> >Environment:
>
> System : OpenBSD 7.4
> Details : OpenBSD 7.4 (GENERIC.MP) #1397: Tue Oct 10 09:02:37
> MDT 2023
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> Architecture: OpenBSD.amd64
> Machine : amd64
>
> >Description:
>
> When calling the "getsockname()" standard C library call, it returns more
> bytes than the actual length in the
>
> resulting socket path. This results in sockets named, for example,
> "socket123", being represented instead as
>
> "socket123\0\0\0\0\0\0\0...". The problem appears to stem from the C library
> implementation setting the socket
>
> name to the maximum length allowed by the socket address rather than the
> actual name. This is not a problem
>
> if you use traditional null-terminated C strings, but becomes a problem in
> other languages which use the returned
>
> length to determine how to use the string.
>
>
> This was found during the course of investigating this issue:
> https://github.com/rust-lang/rust/issues/116523
>
> >How-To-Repeat:
>
> I was able to repeat the bug using this C program:
>
> ---
>
> #include <sys/socket.h>
> #include <sys/un.h>
> #include <errno.h>
> #include <sys/types.h>
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main(void) {
> int server_sock, rc, len, i;
> struct sockaddr_un address_in, address_out;
> char address_name[32] = { 0 };
> char c;
>
> server_sock = socket(AF_UNIX, SOCK_STREAM, 0);
> if (server_sock == -1) {
> puts("failed to open socket");
> exit(1);
> }
>
> address_in.sun_family = AF_UNIX;
> strcpy(address_in.sun_path, "/tmp/socket123");
> len = strlen("/tmp/socket123") + sizeof(address_in) -
> sizeof(address_in.sun_path);
>
> rc = bind(server_sock, &address_in, len);
> if (rc == -1) {
> puts("failed to bind to socket");
> exit(1);
> }
>
> len = 11111;
> rc = getsockname(server_sock, &address_out, &len);
> if (rc == -1) {
> puts("failed to read socket name");
> exit(1);
> }
>
> len -= (sizeof(address_out) - sizeof(address_out.sun_path));
> printf("socket address length is %d\n", len);
>
> for (i = 0; i < len; i++) {
> c = address_out.sun_path[i];
> if (c >= 33 && c <= 126) {
> printf("%c", c);
> } else {
> printf("\\%d", c);
> }
> }
> printf("\n");
>
> close(server_sock);
> return 0;
> }
>
> ---
>
> When run on a Linux machine, it produces this output:
>
> ---
>
> $ cc bug.c -o bug
>
> $ ./bug
>
> socket address length is 15
> /tmp/socket123\0
>
> ---
>
> I receive the same output on FreeBSD and NetBSD.
>
> When run on OpenSBD 7.4, it produces this output:
>
> ---
>
> $ cc bug.c -o bug
>
> $ ./bug
>
> socket address length is 104
> /tmp/socket123\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
>
> \0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0
>
>
> ---
>
> Ideally, OpenBSD would return the same output.
>
> >Fix:
>
> This could be fixed by returning the actual length of the socket address in
> the "len" variable
>
> in the "getsockname" function.
>
>
> I don't believe that dmesg is relevant for this bug; I can provide it if
> requested.
>
I'm not 10%% convinced it is a bug, there's is something to be said
for setting len to sizeof(struct sockaddr_un).
getsockname is underspecified in Posix Unix domain sockets, the other
address families use fixed size structs.
FreeBSD's man page even says it is unsupported for Unix domain
sockets. So I'm undecided, other devs might have stronger opinions.
-Otto
