Information leak in llcp_sock_bind/llcp_raw_sock_bind

[Dmitry Vyukov]

Hello,

The following program leads to leak of unint bytes from kernel stack:

#include <sys/types.h>
#include <sys/socket.h>
#include <linux/in.h>
#include <linux/in6.h>
#include <linux/socket.h>
#include <linux/if.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <fcntl.h>

#define NFC_SOCKPROTO_LLCP 1

int main(void)
{
struct sockaddr sa;
unsigned len, i, try;
int fd;

for (try = 0; try < 3; try++) {
fd = socket(AF_NFC, 3, NFC_SOCKPROTO_LLCP);
if (fd == -1)
return;
switch (try) {
case 0:
break;
case 1:
sched_yield();
break;
case 2:
open("/dev/null", O_RDONLY);
}
memset(&sa, 0, sizeof(sa));
sa.sa_family = AF_NFC;
bind(fd, &sa, 2);
len = sizeof(sa);
getsockname(fd, &sa, &len);
for (i = 0; i < len; i++)
printf("%02x", ((unsigned char*)&sa)[i]);
printf("\n");
}
return 0;
}

Output:
27000000000000000000000000000000b002400000000000001f4511e5e38f900000000000000000b00240000000000018006c00000000007c134000000000000000000000000000000000000100000028f77610fe7f00005e10400000000000
27000000000000000000000000000000b002400000000000000212046c1647690000000000000000b00240000000000018006c00000000007c1340000000000000000000000000000000000001000000c874fff4fe7f00005e10400000000000
27000000000000000000000000000000b002400000000000008e8a91e4e069fc0000000000000000b00240000000000018006c00000000007c1340000000000000000000000000000000000001000000f868b3f2fe7f00005e10400000000000

The problem is that llcp_sock_bind/llcp_raw_sock_bind do not check
sockaddr_len passed in, so they copy stack garbage from stack into the
socket and then return it in getsockname.
This can defeat ASLR, leak crypto keys, etc.

[David Miller]

From: Dmitry Vyukov <dvy...@google.com>
Date: Tue, 15 Dec 2015 21:00:20 +0100

> The problem is that llcp_sock_bind/llcp_raw_sock_bind do not check
> sockaddr_len passed in, so they copy stack garbage from stack into the
> socket and then return it in getsockname.
> This can defeat ASLR, leak crypto keys, etc.

That's actually the first thing these functions do.

They completely clear out the on-stack llcp_addr, then they copy only
as much as the user gave them, being careful not to use more than
sizeof(llcp_addr).

memset(&llcp_addr, 0, sizeof(llcp_addr));
len = min_t(unsigned int, sizeof(llcp_addr), alen);
memcpy(&llcp_addr, addr, len);

I don't see what the problem is, you'll need to be more specific.

Even wrt. llcp_sock->service_name, the code limits the string to
NFC_LLCP_MAX_SERVICE_NAME.

[Dmitry Vyukov]

On Tue, Dec 15, 2015 at 9:36 PM, David Miller <da...@davemloft.net> wrote:
> From: Dmitry Vyukov <dvy...@google.com>
> Date: Tue, 15 Dec 2015 21:00:20 +0100
>
>> The problem is that llcp_sock_bind/llcp_raw_sock_bind do not check
>> sockaddr_len passed in, so they copy stack garbage from stack into the
>> socket and then return it in getsockname.
>> This can defeat ASLR, leak crypto keys, etc.
>
> That's actually the first thing these functions do.
>
> They completely clear out the on-stack llcp_addr, then they copy only
> as much as the user gave them, being careful not to use more than
> sizeof(llcp_addr).
>
> memset(&llcp_addr, 0, sizeof(llcp_addr));
> len = min_t(unsigned int, sizeof(llcp_addr), alen);
> memcpy(&llcp_addr, addr, len);
>
> I don't see what the problem is, you'll need to be more specific.

You are right. Sorry.

There still seems to be a minor leak here:

if (!addr || addr->sa_family != AF_NFC)
return -EINVAL;

addr->sa_family can be uninit.

[David Miller]

From: Dmitry Vyukov <dvy...@google.com>
Date: Tue, 15 Dec 2015 21:45:16 +0100

That shouldn't matter at all, that can't cause socket state corruption.

I want to ask you if you are actually seeing kernel stack in that hexdump
you are posting? If so, how do you actually account for it? Nothing you
have shown so far make that clear.

[Dmitry Vyukov]

I've seen a kernel address at least in pptp_bind, it was a return pc
in SyS_socket call that was executed just before bind.
Exact contents of the leaked info depend on kernel config, compiler
and a previous executed syscall (there are thousands of them if we
count ioctls and friends). So it is almost impossible to prove that a
PC cannot be leaked.

[David Miller]

From: Dmitry Vyukov <dvy...@google.com>
Date: Tue, 15 Dec 2015 21:55:37 +0100

> I've seen a kernel address at least in pptp_bind,

We're not talking about pptp_bind.

We're talking about llcp_{,raw}_sock_bind().

If your hex dump doesn't show it, don't report anything unless you are
absolutely sure via code inspection that there could be a leak. And
in that case make it perfectly clear exactly how that can happen.

I am generally unimpressed with your reports half of the time,
and just a small amount of extra effort would extraordinarily
improve the quality of the things your post.

Thanks.

> So it is almost impossible to prove that a PC cannot be leaked.

You can't show that anything is actually being leaked in this specific
case, period.

[Dmitry Vyukov]

I am a human and sometimes do mistakes.

In this case, I checked that the bind succeeds when I pass
sockaddrlen=0, which is suspicious and matches the behavior of pptp
case (not checking sockaddrlen at all). Then I looked at the code and
misread it, because it uses a different idiom from other cases I saw
(explicitly checking sockaddrlen value). Then I wrote a test program
and observed a varying, garbage-looking values returned from
getsockname. From that I concluded that there is an information leak.
This is wrong.

For the purpose of improvement of my reports, what are the other
reports you are not impressed with and why?

Thank you

[jduck]

FWIW, the LLCP case seems to be CVE-2013-3233 - http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d26d6504f23e803824e8ebd14e52d4fc0a0b09cb