Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1292512 > unrolled thread

Information leak in llcp_sock_bind/llcp_raw_sock_bind

Back to article view | Back to linux.kernel

Contents

  Information leak in llcp_sock_bind/llcp_raw_sock_bind Dmitry Vyukov <dvyukov@google.com> - 2015-12-15 21:10 +0100
    Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind David Miller <davem@davemloft.net> - 2015-12-15 21:40 +0100
      Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind David Miller <davem@davemloft.net> - 2015-12-15 21:50 +0100
        Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind David Miller <davem@davemloft.net> - 2015-12-15 22:00 +0100
          Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind Dmitry Vyukov <dvyukov@google.com> - 2015-12-16 20:10 +0100
        Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind Dmitry Vyukov <dvyukov@google.com> - 2015-12-15 22:00 +0100
      Re: Information leak in llcp_sock_bind/llcp_raw_sock_bind Dmitry Vyukov <dvyukov@google.com> - 2015-12-15 21:50 +0100

#1292512 — Information leak in llcp_sock_bind/llcp_raw_sock_bind

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1292526

From: Dmitry Vyukov <dvyukov@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292537

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

> On Tue, Dec 15, 2015 at 9:36 PM, David Miller <davem@davemloft.net> wrote:
>> From: Dmitry Vyukov <dvyukov@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.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292540

From: Dmitry Vyukov <dvyukov@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1293314

On Tue, Dec 15, 2015 at 9:58 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@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.


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292541

On Tue, Dec 15, 2015 at 9:48 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@google.com>
> Date: Tue, 15 Dec 2015 21:45:16 +0100
>
>> On Tue, Dec 15, 2015 at 9:36 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Dmitry Vyukov <dvyukov@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.
>
> 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.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292539

On Tue, Dec 15, 2015 at 9:36 PM, David Miller <davem@davemloft.net> wrote:
> From: Dmitry Vyukov <dvyukov@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web