netdev - Re: [PATCH] af_unix: Add sockaddr length checks before accessing sa

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]

Date:   Fri, 9 Jun 2017 13:15:40 +0200
From:   Mateusz Jurczyk <mjurczyk@...gle.com>
To:     David Miller <davem@...emloft.net>
Cc:     WANG Cong <xiyou.wangcong@...il.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Kees Cook <keescook@...omium.org>,
        Miklos Szeredi <mszeredi@...hat.com>,
        Isaac Boukris <iboukris@...il.com>,
        Andrey Vagin <avagin@...nvz.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] af_unix: Add sockaddr length checks before accessing
 sa_family in bind and connect handlers

On Thu, Jun 8, 2017 at 10:04 PM, David Miller <davem@...emloft.net> wrote:
> From: Mateusz Jurczyk <mjurczyk@...gle.com>
> Date: Thu,  8 Jun 2017 11:13:36 +0200
>
>> Verify that the caller-provided sockaddr structure is large enough to
>> contain the sa_family field, before accessing it in bind() and connect()
>> handlers of the AF_UNIX socket. Since neither syscall enforces a minimum
>> size of the corresponding memory region, very short sockaddrs (zero or
>> one byte long) result in operating on uninitialized memory while
>> referencing .sa_family.
>>
>> Signed-off-by: Mateusz Jurczyk <mjurczyk@...gle.com>
>
> The sockaddr comes from a structure on the caller's kernel stack, even
> if the user gives a smaller length, it is legal to access that memory.

It is legal to access it, but since it's uninitialized kernel stack
memory, the results of comparisons against AF_UNIX or AF_UNSPEC are
indeterminate. In practice a user-mode program could likely use timing
measurement to infer the evaluation of these comparisons, and hence
determine if a garbage 16-bit variable on the kernel stack is equal to
0x0000 or 0x0001, or a garbage byte is equal to 0x00 (if the first
byte is provided).

This is of course not very bad. However, my project for finding use of
uninitialized memory flagged it, and I thought it was worth fixing, at
least to avoid having this construct detected in the future (e.g. by
KMSAN).

There are a few more instances of this behavior in other socket types,
which I was going to report with separate patches. If you decide this
kind of issues indeed deserves a fix, please let me know if further
separate patches are the right approach.

Thanks,
Mateusz