2012-08-15 21:32:59

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 00/14] net: info leaks and other bugs

Hi David,

this series fixes quite a bunch of info leaks under net/. There is also
one NULL pointer deref fix ("dccp: check ccid before..") that could be
abused for privilege escalation.

The info leak fixes might be material for stable, too. But I leave the
decision up to you.

On request, test code for all (but one) of the issues can be provided.


Regards,
Mathias


Mathias Krause (14):
atm: fix info leak in getsockopt(SO_ATMPVC)
atm: fix info leak via getsockname()
Bluetooth: HCI - Fix info leak in getsockopt(HCI_FILTER)
Bluetooth: HCI - Fix info leak via getsockname()
Bluetooth: RFCOMM - Fix info leak in getsockopt(BT_SECURITY)
Bluetooth: RFCOMM - Fix info leak in ioctl(RFCOMMGETDEVLIST)
Bluetooth: RFCOMM - Fix info leak via getsockname()
Bluetooth: L2CAP - Fix info leak via getsockname()
l2tp: fix info leak via getsockname()
llc: fix info leak via getsockname()
dccp: check ccid before dereferencing
dccp: fix info leak via getsockopt(DCCP_SOCKOPT_CCID_TX_INFO)
ipvs: fix info leak in getsockopt(IP_VS_SO_GET_TIMEOUT)
net: fix info leak in compat dev_ifconf()

net/atm/common.c | 1 +
net/atm/pvc.c | 1 +
net/bluetooth/hci_sock.c | 2 ++
net/bluetooth/l2cap_sock.c | 1 +
net/bluetooth/rfcomm/sock.c | 2 ++
net/bluetooth/rfcomm/tty.c | 2 +-
net/dccp/ccid.h | 4 ++--
net/dccp/ccids/ccid3.c | 1 +
net/l2tp/l2tp_ip6.c | 1 +
net/llc/af_llc.c | 3 +--
net/netfilter/ipvs/ip_vs_ctl.c | 1 +
net/socket.c | 1 +
12 files changed, 15 insertions(+), 5 deletions(-)

--
1.7.10.4


2012-08-15 21:33:01

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 01/14] atm: fix info leak in getsockopt(SO_ATMPVC)

The ATM code fails to initialize the two padding bytes of struct
sockaddr_atmpvc inserted for alignment. Add an explicit memset(0)
before filling the structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
---
net/atm/common.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/atm/common.c b/net/atm/common.c
index b4b44db..0c0ad93 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -812,6 +812,7 @@ int vcc_getsockopt(struct socket *sock, int level, int optname,

if (!vcc->dev || !test_bit(ATM_VF_ADDR, &vcc->flags))
return -ENOTCONN;
+ memset(&pvc, 0, sizeof(pvc));
pvc.sap_family = AF_ATMPVC;
pvc.sap_addr.itf = vcc->dev->number;
pvc.sap_addr.vpi = vcc->vpi;
--
1.7.10.4

2012-08-15 21:33:13

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 06/14] Bluetooth: RFCOMM - Fix info leak in ioctl(RFCOMMGETDEVLIST)

The RFCOMM code fails to initialize the two padding bytes of struct
rfcomm_dev_list_req inserted for alignment before copying it to
userland. Additionally there are two padding bytes in each instance of
struct rfcomm_dev_info. The ioctl() that for disclosures two bytes plus
dev_num times two bytes uninitialized kernel heap memory.

Allocate the memory using kzalloc() to fix this issue.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/rfcomm/tty.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index cb96077..56f1823 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -456,7 +456,7 @@ static int rfcomm_get_dev_list(void __user *arg)

size = sizeof(*dl) + dev_num * sizeof(*di);

- dl = kmalloc(size, GFP_KERNEL);
+ dl = kzalloc(size, GFP_KERNEL);
if (!dl)
return -ENOMEM;

--
1.7.10.4

2012-08-15 21:33:17

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 10/14] llc: fix info leak via getsockname()

The LLC code wrongly returns 0, i.e. "success", when the socket is
zapped. Together with the uninitialized uaddrlen pointer argument from
sys_getsockname this leads to an arbitrary memory leak of up to 128
bytes kernel stack via the getsockname() syscall.

Return an error instead when the socket is zapped to prevent the info
leak. Also remove the unnecessary memset(0). We don't directly write to
the memory pointed by uaddr but memcpy() a local structure at the end of
the function that is properly initialized.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
net/llc/af_llc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index f6fe4d4..526f454 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -969,14 +969,13 @@ static int llc_ui_getname(struct socket *sock, struct sockaddr *uaddr,
struct sockaddr_llc sllc;
struct sock *sk = sock->sk;
struct llc_sock *llc = llc_sk(sk);
- int rc = 0;
+ int rc = -EBADF;

memset(&sllc, 0, sizeof(sllc));
lock_sock(sk);
if (sock_flag(sk, SOCK_ZAPPED))
goto out;
*uaddrlen = sizeof(sllc);
- memset(uaddr, 0, *uaddrlen);
if (peer) {
rc = -ENOTCONN;
if (sk->sk_state != TCP_ESTABLISHED)
--
1.7.10.4

2012-08-15 21:33:26

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 14/14] net: fix info leak in compat dev_ifconf()

The implementation of dev_ifconf() for the compat ioctl interface uses
an intermediate ifc structure allocated in userland for the duration of
the syscall. Though, it fails to initialize the padding bytes inserted
for alignment and that for leaks four bytes of kernel stack. Add an
explicit memset(0) before filling the structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
---
net/socket.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index dfe5b66..a5471f8 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2657,6 +2657,7 @@ static int dev_ifconf(struct net *net, struct compat_ifconf __user *uifc32)
if (copy_from_user(&ifc32, uifc32, sizeof(struct compat_ifconf)))
return -EFAULT;

+ memset(&ifc, 0, sizeof(ifc));
if (ifc32.ifcbuf == 0) {
ifc32.ifc_len = 0;
ifc.ifc_len = 0;
--
1.7.10.4

2012-08-15 21:33:23

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 12/14] dccp: fix info leak via getsockopt(DCCP_SOCKOPT_CCID_TX_INFO)

The CCID3 code fails to initialize the trailing padding bytes of struct
tfrc_tx_info added for alignment on 64 bit architectures. It that for
potentially leaks four bytes kernel stack via the getsockopt() syscall.
Add an explicit memset(0) before filling the structure to avoid the
info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Gerrit Renker <[email protected]>
---
net/dccp/ccids/ccid3.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index d65e987..119c043 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -535,6 +535,7 @@ static int ccid3_hc_tx_getsockopt(struct sock *sk, const int optname, int len,
case DCCP_SOCKOPT_CCID_TX_INFO:
if (len < sizeof(tfrc))
return -EINVAL;
+ memset(&tfrc, 0, sizeof(tfrc));
tfrc.tfrctx_x = hc->tx_x;
tfrc.tfrctx_x_recv = hc->tx_x_recv;
tfrc.tfrctx_x_calc = hc->tx_x_calc;
--
1.7.10.4

2012-08-15 21:34:01

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 13/14] ipvs: fix info leak in getsockopt(IP_VS_SO_GET_TIMEOUT)

If at least one of CONFIG_IP_VS_PROTO_TCP or CONFIG_IP_VS_PROTO_UDP is
not set, __ip_vs_get_timeouts() does not fully initialize the structure
that gets copied to userland and that for leaks up to 12 bytes of kernel
stack. Add an explicit memset(0) before passing the structure to
__ip_vs_get_timeouts() to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Wensong Zhang <[email protected]>
Cc: Simon Horman <[email protected]>
Cc: Julian Anastasov <[email protected]>
---
net/netfilter/ipvs/ip_vs_ctl.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 84444dd..72bf32a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2759,6 +2759,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
{
struct ip_vs_timeout_user t;

+ memset(&t, 0, sizeof(t));
__ip_vs_get_timeouts(net, &t);
if (copy_to_user(user, &t, sizeof(t)) != 0)
ret = -EFAULT;
--
1.7.10.4

2012-08-15 21:34:36

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 11/14] dccp: check ccid before dereferencing

ccid_hc_rx_getsockopt() and ccid_hc_tx_getsockopt() might be called with
a NULL ccid pointer leading to a NULL pointer dereference. This could
lead to a privilege escalation if the attacker is able to map page 0 and
prepare it with a fake ccid_ops pointer.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Gerrit Renker <[email protected]>
Cc: [email protected]
---
net/dccp/ccid.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dccp/ccid.h b/net/dccp/ccid.h
index 75c3582..fb85d37 100644
--- a/net/dccp/ccid.h
+++ b/net/dccp/ccid.h
@@ -246,7 +246,7 @@ static inline int ccid_hc_rx_getsockopt(struct ccid *ccid, struct sock *sk,
u32 __user *optval, int __user *optlen)
{
int rc = -ENOPROTOOPT;
- if (ccid->ccid_ops->ccid_hc_rx_getsockopt != NULL)
+ if (ccid != NULL && ccid->ccid_ops->ccid_hc_rx_getsockopt != NULL)
rc = ccid->ccid_ops->ccid_hc_rx_getsockopt(sk, optname, len,
optval, optlen);
return rc;
@@ -257,7 +257,7 @@ static inline int ccid_hc_tx_getsockopt(struct ccid *ccid, struct sock *sk,
u32 __user *optval, int __user *optlen)
{
int rc = -ENOPROTOOPT;
- if (ccid->ccid_ops->ccid_hc_tx_getsockopt != NULL)
+ if (ccid != NULL && ccid->ccid_ops->ccid_hc_tx_getsockopt != NULL)
rc = ccid->ccid_ops->ccid_hc_tx_getsockopt(sk, optname, len,
optval, optlen);
return rc;
--
1.7.10.4

2012-08-15 21:35:19

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 09/14] l2tp: fix info leak via getsockname()

The L2TP code for IPv6 fails to initialize the l2tp_unused member of
struct sockaddr_l2tpip6 and that for leaks two bytes kernel stack via
the getsockname() syscall. Initialize l2tp_unused with 0 to avoid the
info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: James Chapman <[email protected]>
---
net/l2tp/l2tp_ip6.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 35e1e4b..9275471 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -410,6 +410,7 @@ static int l2tp_ip6_getname(struct socket *sock, struct sockaddr *uaddr,
lsa->l2tp_family = AF_INET6;
lsa->l2tp_flowinfo = 0;
lsa->l2tp_scope_id = 0;
+ lsa->l2tp_unused = 0;
if (peer) {
if (!lsk->peer_conn_id)
return -ENOTCONN;
--
1.7.10.4

2012-08-15 21:33:10

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 04/14] Bluetooth: HCI - Fix info leak via getsockname()

The HCI code fails to initialize the hci_channel member of struct
sockaddr_hci and that for leaks two bytes kernel stack via the
getsockname() syscall. Initialize hci_channel with 0 to avoid the
info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_sock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index a27bbc3..19fdac7 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -694,6 +694,7 @@ static int hci_sock_getname(struct socket *sock, struct sockaddr *addr,
*addr_len = sizeof(*haddr);
haddr->hci_family = AF_BLUETOOTH;
haddr->hci_dev = hdev->id;
+ haddr->hci_channel= 0;

release_sock(sk);
return 0;
--
1.7.10.4

2012-08-15 21:35:49

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 08/14] Bluetooth: L2CAP - Fix info leak via getsockname()

The L2CAP code fails to initialize the l2_bdaddr_type member of struct
sockaddr_l2 and the padding byte added for alignment. It that for leaks
two bytes kernel stack via the getsockname() syscall. Add an explicit
memset(0) before filling the structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/l2cap_sock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index a4bb27e..df5ea9e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -245,6 +245,7 @@ static int l2cap_sock_getname(struct socket *sock, struct sockaddr *addr, int *l

BT_DBG("sock %p, sk %p", sock, sk);

+ memset(la, 0, sizeof(struct sockaddr_l2));
addr->sa_family = AF_BLUETOOTH;
*len = sizeof(struct sockaddr_l2);

--
1.7.10.4

2012-08-15 21:36:24

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 07/14] Bluetooth: RFCOMM - Fix info leak via getsockname()

The RFCOMM code fails to initialize the trailing padding byte of struct
sockaddr_rc added for alignment. It that for leaks one byte kernel stack
via the getsockname() syscall. Add an explicit memset(0) before filling
the structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 64f55ca..1a17850 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -528,6 +528,7 @@ static int rfcomm_sock_getname(struct socket *sock, struct sockaddr *addr, int *

BT_DBG("sock %p, sk %p", sock, sk);

+ memset(sa, 0, sizeof(*sa));
sa->rc_family = AF_BLUETOOTH;
sa->rc_channel = rfcomm_pi(sk)->channel;
if (peer)
--
1.7.10.4

2012-08-15 21:33:07

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 03/14] Bluetooth: HCI - Fix info leak in getsockopt(HCI_FILTER)

The HCI code fails to initialize the two padding bytes of struct
hci_ufilter before copying it to userland -- that for leaking two
bytes kernel stack. Add an explicit memset(0) before filling the
structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_sock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index a7f04de..a27bbc3 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -1009,6 +1009,7 @@ static int hci_sock_getsockopt(struct socket *sock, int level, int optname,
{
struct hci_filter *f = &hci_pi(sk)->filter;

+ memset(&uf, 0, sizeof(uf));
uf.type_mask = f->type_mask;
uf.opcode = f->opcode;
uf.event_mask[0] = *((u32 *) f->event_mask + 0);
--
1.7.10.4

2012-08-15 21:36:56

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 05/14] Bluetooth: RFCOMM - Fix info leak in getsockopt(BT_SECURITY)

The RFCOMM code fails to initialize the key_size member of struct
bt_security before copying it to userland -- that for leaking one
byte kernel stack. Initialize key_size with 0 to avoid the info
leak.

Signed-off-by: Mathias Krause <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 7e1e596..64f55ca 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -822,6 +822,7 @@ static int rfcomm_sock_getsockopt(struct socket *sock, int level, int optname, c
}

sec.level = rfcomm_pi(sk)->sec_level;
+ sec.key_size = 0;

len = min_t(unsigned int, len, sizeof(sec));
if (copy_to_user(optval, (char *) &sec, len))
--
1.7.10.4

2012-08-15 21:37:38

by Mathias Krause

[permalink] [raw]
Subject: [PATCH 02/14] atm: fix info leak via getsockname()

The ATM code fails to initialize the two padding bytes of struct
sockaddr_atmpvc inserted for alignment. Add an explicit memset(0)
before filling the structure to avoid the info leak.

Signed-off-by: Mathias Krause <[email protected]>
---
net/atm/pvc.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/atm/pvc.c b/net/atm/pvc.c
index 3a73491..ae03240 100644
--- a/net/atm/pvc.c
+++ b/net/atm/pvc.c
@@ -95,6 +95,7 @@ static int pvc_getname(struct socket *sock, struct sockaddr *sockaddr,
return -ENOTCONN;
*sockaddr_len = sizeof(struct sockaddr_atmpvc);
addr = (struct sockaddr_atmpvc *)sockaddr;
+ memset(addr, 0, sizeof(*addr));
addr->sap_family = AF_ATMPVC;
addr->sap_addr.itf = vcc->dev->number;
addr->sap_addr.vpi = vcc->vpi;
--
1.7.10.4

2012-08-16 04:40:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 00/14] net: info leaks and other bugs

From: Mathias Krause <[email protected]>
Date: Wed, 15 Aug 2012 23:31:43 +0200

> this series fixes quite a bunch of info leaks under net/. There is also
> one NULL pointer deref fix ("dccp: check ccid before..") that could be
> abused for privilege escalation.
>
> The info leak fixes might be material for stable, too. But I leave the
> decision up to you.
>
> On request, test code for all (but one) of the issues can be provided.

All applied and queued up for -stable, thanks a lot.