netdev - Re: [PATCH] af_unix: Escape abstract unix socket address

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220406145941.728b4cb5@hermes.local>
Date:   Wed, 6 Apr 2022 14:59:41 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Yann Droneaud <ydroneaud@...eya.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] af_unix: Escape abstract unix socket address

On Wed,  6 Apr 2022 12:22:13 +0200
Yann Droneaud <ydroneaud@...eya.com> 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 <ydroneaud@...eya.com>
> ---
>  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.