2022-04-06 15:24:02

by Yann Droneaud

[permalink] [raw]
Subject: [PATCH] af_unix: Escape abstract unix socket address

Abstract unix socket address are bytes sequences up to
108 bytes (UNIX_PATH_MAX == sizeof(struct sockaddr_un) -
offsetof(struct sockaddr_un, sun_path)).

As with any random string of bytes, printing them in
/proc/net/unix should be done with caution to prevent
misbehavior.

It would have been great to use seq_escape_mem() to escape
the control characters in a reversible way.

Unfortunately userspace might expect that NUL bytes are
replaced with '@' characters as it's done currently.

So this patch implements the following scheme: any control
characters, including NUL, in the abstract unix socket
addresses is replaced by '@' characters.

Sadly, with such non reversible escape scheme, abstract
addresses such as "\0\0", "\0\a", "\0\b", "\0\t", etc.
will have the same representation: "@@".

But will prevent "cat /proc/net/unix" from messing with
terminal, and will prevent "\n" in abstract address from
messing with parsing the list of Unix sockets.

Signed-off-by: Yann Droneaud <[email protected]>
---
net/unix/af_unix.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e71a312faa1e..8021efd92301 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -3340,7 +3340,8 @@ static int unix_seq_show(struct seq_file *seq, void *v)
i++;
}
for ( ; i < len; i++)
- seq_putc(seq, u->addr->name->sun_path[i] ?:
+ seq_putc(seq, !iscntrl(u->addr->name->sun_path[i]) ?
+ u->addr->name->sun_path[i] :
'@');
}
unix_state_unlock(s);
--
2.32.0


2022-04-07 01:31:39

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH] af_unix: Escape abstract unix socket address

On Wed, 6 Apr 2022 12:22:13 +0200
Yann Droneaud <[email protected]> wrote:

> Abstract unix socket address are bytes sequences up to
> 108 bytes (UNIX_PATH_MAX == sizeof(struct sockaddr_un) -
> offsetof(struct sockaddr_un, sun_path)).
>
> As with any random string of bytes, printing them in
> /proc/net/unix should be done with caution to prevent
> misbehavior.
>
> It would have been great to use seq_escape_mem() to escape
> the control characters in a reversible way.
>
> Unfortunately userspace might expect that NUL bytes are
> replaced with '@' characters as it's done currently.
>
> So this patch implements the following scheme: any control
> characters, including NUL, in the abstract unix socket
> addresses is replaced by '@' characters.
>
> Sadly, with such non reversible escape scheme, abstract
> addresses such as "\0\0", "\0\a", "\0\b", "\0\t", etc.
> will have the same representation: "@@".
>
> But will prevent "cat /proc/net/unix" from messing with
> terminal, and will prevent "\n" in abstract address from
> messing with parsing the list of Unix sockets.
>
> Signed-off-by: Yann Droneaud <[email protected]>
> ---
> net/unix/af_unix.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index e71a312faa1e..8021efd92301 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -3340,7 +3340,8 @@ static int unix_seq_show(struct seq_file *seq, void *v)
> i++;
> }
> for ( ; i < len; i++)
> - seq_putc(seq, u->addr->name->sun_path[i] ?:
> + seq_putc(seq, !iscntrl(u->addr->name->sun_path[i]) ?
> + u->addr->name->sun_path[i] :
> '@');
> }
> unix_state_unlock(s);

Unfortunately, you will break userspace ABI with this.

2022-04-07 20:22:48

by Yann Droneaud

[permalink] [raw]
Subject: Re: [PATCH] af_unix: Escape abstract unix socket address

Hi,

Le 06/04/2022 à 23:59, Stephen Hemminger a écrit :
> On Wed, 6 Apr 2022 12:22:13 +0200
> Yann Droneaud <[email protected]> wrote:
>
>> Abstract unix socket address are bytes sequences up to
>> 108 bytes (UNIX_PATH_MAX == sizeof(struct sockaddr_un) -
>> offsetof(struct sockaddr_un, sun_path)).
>>
>> As with any random string of bytes, printing them in
>> /proc/net/unix should be done with caution to prevent
>> misbehavior.
>>
>> It would have been great to use seq_escape_mem() to escape
>> the control characters in a reversible way.
>>
>> Unfortunately userspace might expect that NUL bytes are
>> replaced with '@' characters as it's done currently.
>>
>> So this patch implements the following scheme: any control
>> characters, including NUL, in the abstract unix socket
>> addresses is replaced by '@' characters.
>>
>> Sadly, with such non reversible escape scheme, abstract
>> addresses such as "\0\0", "\0\a", "\0\b", "\0\t", etc.
>> will have the same representation: "@@".
>>
>> But will prevent "cat /proc/net/unix" from messing with
>> terminal, and will prevent "\n" in abstract address from
>> messing with parsing the list of Unix sockets.
>>
>> Signed-off-by: Yann Droneaud <[email protected]>
>> ---
>> net/unix/af_unix.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index e71a312faa1e..8021efd92301 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -3340,7 +3340,8 @@ static int unix_seq_show(struct seq_file *seq, void *v)
>> i++;
>> }
>> for ( ; i < len; i++)
>> - seq_putc(seq, u->addr->name->sun_path[i] ?:
>> + seq_putc(seq, !iscntrl(u->addr->name->sun_path[i]) ?
>> + u->addr->name->sun_path[i] :
>> '@');
>> }
>> unix_state_unlock(s);
> Unfortunately, you will break userspace ABI with this.

It's a wanted side effect.

Consider the following program


#include <stddef.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>

#define ADDRESS "\0\n0000000000000000: 00000003 00000000 00000000 0001 03 1234567890 /bin/true"

int main(void)
{
static const struct sockaddr_un un = {
.sun_family = AF_UNIX,
.sun_path = ADDRESS,
};
int s;

s = socket(AF_UNIX, SOCK_STREAM, 0);
if (s < 0) {
perror("socket");
return 1;
}

if (bind(s, (const struct sockaddr *)&un, offsetof(struct sockaddr_un,sun_path) + sizeof(ADDRESS) - 1) < 0) {
perror("bind");
return 1;
}

while (1)
pause();

return 0;
}


This confuses
- cat /proc/net/unix
- netstat -x

Only ss -xl doesn't take /bin/true as a Unix socket (but ss output is broken because it doesn't escape \n in unix addresses)


Regards.

--
Yann Droneaud
OPTEYA