2007-03-23 13:42:36

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] fix information leak in wireless extensions on 64-bit platforms

Wireless extensions on 64-bit platforms leak information from the kernel
stack due to padding in structs that is copied. This affects any
wireless event stream including scan results and so hence is available
to unprivileged users.

This patch is a quick fix for this that simply zeroes out the padding in
the structs before copying them until Jean comes up with the promised
better fix, at which time he can revert this one along with his new
patch.

Signed-off-by: Johannes Berg <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Jean Tourrilhes <[email protected]>

---
include/net/iw_handler.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

--- wireless-dev.orig/include/net/iw_handler.h 2007-03-17 20:19:45.369309540 +0100
+++ wireless-dev/include/net/iw_handler.h 2007-03-17 20:19:59.429309540 +0100
@@ -484,6 +484,9 @@ iwe_stream_add_event(char * stream, /*
struct iw_event *iwe, /* Payload */
int event_len) /* Real size of payload */
{
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Check if it's possible */
if(likely((stream + event_len) < ends)) {
iwe->len = event_len;
@@ -505,6 +508,10 @@ iwe_stream_add_point(char * stream, /*
char * extra) /* More payload */
{
int event_len = IW_EV_POINT_LEN + iwe->u.data.length;
+
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Check if it's possible */
if(likely((stream + event_len) < ends)) {
iwe->len = event_len;
@@ -531,6 +538,9 @@ iwe_stream_add_value(char * event, /* E
struct iw_event *iwe, /* Payload */
int event_len) /* Real size of payload */
{
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Don't duplicate LCP */
event_len -= IW_EV_LCP_LEN;

@@ -558,6 +568,9 @@ iwe_stream_check_add_event(char * stream
int event_len, /* Size of payload */
int * perr) /* Error report */
{
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Check if it's possible, set error if not */
if(likely((stream + event_len) < ends)) {
iwe->len = event_len;
@@ -582,6 +595,10 @@ iwe_stream_check_add_point(char * stream
int * perr) /* Error report */
{
int event_len = IW_EV_POINT_LEN + iwe->u.data.length;
+
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Check if it's possible */
if(likely((stream + event_len) < ends)) {
iwe->len = event_len;
@@ -611,6 +628,9 @@ iwe_stream_check_add_value(char * event,
int event_len, /* Size of payload */
int * perr) /* Error report */
{
+ /* clear padding */
+ memset((char*)iwe + 4, 0, IW_EV_LCP_LEN - 4);
+
/* Don't duplicate LCP */
event_len -= IW_EV_LCP_LEN;





2007-03-23 16:59:23

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Friday 23 March 2007 17:13, Jean Tourrilhes wrote:
> value. It seems that it's too late for the next release of Debian or
> Fedora,

Wtf? It's too late for a security fix?
How can it be too late for a security fix?

--
Greetings Michael.

2007-03-23 15:59:44

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, 2007-03-23 at 08:48 -0700, Jean Tourrilhes wrote:

> Yes, sorry for the delay, I wanted to make sure I had all the
> parts fully fleshed out.

:)

Could you maybe send yours to stable too? Both of them I guess. Or do
you want us (John I guess) to do that?

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-03-23 16:24:57

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 11:42:37AM -0400, John W. Linville wrote:
> On Fri, Mar 23, 2007 at 02:42:42PM +0100, Johannes Berg wrote:
> > On Fri, 2007-03-23 at 14:40 +0100, Johannes Berg wrote:
> > > Wireless extensions on 64-bit platforms leak information from the kernel
> > > stack due to padding in structs that is copied. This affects any
> > > wireless event stream including scan results and so hence is available
> > > to unprivileged users.
> >
> > Ignore this patch, I see Jean just posted one as well :)
>
> Hmmm...I seem to have missed it. Perhaps I was too quick w/ the 'd'
> this morning? :-)

You too are getting far too much spam ?

> Jean, could you bounce me another copy?

No problem. Original e-mails here, patches attached :
http://marc.info/?l=linux-netdev&m=117460953806538&w=2
http://marc.info/?l=linux-netdev&m=117460988706771&w=2
Note that you decide if you want this in stable as well...

> Thanks,
>
> John

Have fun...

Jean


Attachments:
(No filename) (964.00 B)
iw261_wpa_compat_ioctl.diff (1.47 kB)
iw261_we22_64bit_usp_leak-3.diff (14.76 kB)
Download all attachments

2007-03-23 20:28:14

by Michael Büsch

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Friday 23 March 2007 21:00, Jean Tourrilhes wrote:
> On Fri, Mar 23, 2007 at 12:36:43PM -0700, Chris Wright wrote:
> > * Jean Tourrilhes ([email protected]) wrote:
> >
> > > The last thing is that I could not test those stuff myself,
> > > and the testing done for me was limited. I would be more confident if
> > > you were to give it a good beating before we push that into stable,
> > > because you know what to test.
> >
> > That's useful info. Have you had any luck (it looks pretty straight
> > forward)?
>
> Let's be more clear. I tested properly on 32 bits. But, 32
> bits is not where there is issue, this issue is specific to 64 bits. I
> don't have a 64 bits platfor. It seems that few people doing wireless
> have access to 64 bits platform (actually, it seems most of them don't
> have access to SMP either), otherwise this would have been found many
> years ago.
> Johannes is one of the few having access to 64 bits stuff.

I finally have an USB ZD stick now, too. So I can test stuff, too.
Should I test this somehow?

--
Greetings Michael.

2007-03-24 16:51:29

by Michael Büsch

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Friday 23 March 2007 21:51, Jean Tourrilhes wrote:
> On Fri, Mar 23, 2007 at 09:27:17PM +0100, Michael Buesch wrote:
> > On Friday 23 March 2007 21:00, Jean Tourrilhes wrote:
> >
> > I finally have an USB ZD stick now, too. So I can test stuff, too.
> > Should I test this somehow?
>
> Ok, here it goes.
> Pick the latest version of wtools. You may want to compile it
> static to avoid the need to install it.
>
> To test the wpa patch, you need 32 bit userspace on 64 bit
> kernel, and you need to try :
> 'iwlist genie'
>
> To test the leak patch, you need a 64 kernel and any

Ok, so the most important question: Where to get the latest
versions of these patches? :)
I kind of lost track of what was agreed on and what's latest, etc..
Some link to the mail?

> userspace. You need two changes to enable the debugging code.
> 1) Put '#define DEBUG 1' on top of iwlib.c.
> 2) Change '#if 0' to '#if 1' line 783 of iwlist.c, at the
> bottom of print_scanning_info().
> Compile everything.
> If you do a 'iwlist scan', it should dump the full content of
> the stream. The first 4 bytes is the header (length + type). The next
> 4 bytes on 64 bits should be all zero.
> This is what I don't want to see :
> [19:00:1B:8B:50:8A:35:E0:09:00:01:00:50:8A:35:F0:47:6F:6C:6F:73:4E:65:74:7A
> This is what I want to see :
> [19:00:1B:8B:00:00:00:00:09:00:01:00:50:8A:35:F0:47:6F:6C:6F:73:4E:65:74:7A
> Note that du to endian considerations you may have some of
> your bytes swapped.
>
> > Greetings Michael.
>
> Good luck...
>
> Jean
>
>

--
Greetings Michael.

2007-03-23 17:59:49

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 05:53:31PM +0100, Michael Buesch wrote:
> On Friday 23 March 2007 17:13, Jean Tourrilhes wrote:
> > value. It seems that it's too late for the next release of Debian or
> > Fedora,
>
> Wtf? It's too late for a security fix?
> How can it be too late for a security fix?

Note that I was making a prediction. We'll see if I'm right.

Let's not make blanket statements like this about security,
security is all about level of risk, there are various level of
"security issues" and you need to assign the proper level to this one.
One one hand of the scale you have issues that allow remote
penetration. Those require immediate attention.
On the other end of the scale you have random information
leaks. Those are clearly important, but clearly not in the same
category. They don't allow remote penetration. They don't allow
priviledge escalation. They don't allow denial of service. The 4 bytes
leaked are comming from mostly random allocated buffers. The potential
of exploitation is very limited.

Both the Debian release and Fedora release are well into their
respective freeze. In particular, the Debian kernel is frozen and
won't change until release. With the amount of issues and open bugs
those kernel packagers have, everything is prioritised, and many
things in their queue tend to be ignored.
The priority those maintainers will assign to this issue will
mostly go along the lines outlined above. Risk of changes and
potential regression versus risk of attack.
This is why I made this prediction.

> Greetings Michael.

Jean

2007-03-23 15:50:28

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 02:42:42PM +0100, Johannes Berg wrote:
> On Fri, 2007-03-23 at 14:40 +0100, Johannes Berg wrote:
> > Wireless extensions on 64-bit platforms leak information from the kernel
> > stack due to padding in structs that is copied. This affects any
> > wireless event stream including scan results and so hence is available
> > to unprivileged users.
>
> Ignore this patch, I see Jean just posted one as well :)
>
> johannes

Yes, sorry for the delay, I wanted to make sure I had all the
parts fully fleshed out.
Thanks !

Jean


2007-03-23 13:44:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, 2007-03-23 at 14:40 +0100, Johannes Berg wrote:
> Wireless extensions on 64-bit platforms leak information from the kernel
> stack due to padding in structs that is copied. This affects any
> wireless event stream including scan results and so hence is available
> to unprivileged users.

Ignore this patch, I see Jean just posted one as well :)

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-03-23 16:37:41

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 09:13:39AM -0700, Jean Tourrilhes wrote:
> On Fri, Mar 23, 2007 at 04:56:24PM +0100, Johannes Berg wrote:
> > On Fri, 2007-03-23 at 08:48 -0700, Jean Tourrilhes wrote:
> >
> > > Yes, sorry for the delay, I wanted to make sure I had all the
> > > parts fully fleshed out.
> >
> > :)
> >
> > Could you maybe send yours to stable too? Both of them I guess. Or do
> > you want us (John I guess) to do that?
> >
> > johannes
>
> It seems that the lifetime of each stable branch is measured
> in weeks (apart from 2.6.16), and being an old timer, gaining a few
> weeks does not seem to me to be worth the trouble. It's not like the
> bug is entirely new.
> I'm personally more concerned about distro. If getting it into
> stable would make it go into a distro release, then I would see more
> value. It seems that it's too late for the next release of Debian or
> Fedora, and I don't know if any distro is going to be based on this
> stable branch.

Lots of distros watch the stable branches to see what they need to pick
up for their kernel releases.

So please don't just ignore them, lots of people depend on them, even
outside of the distros.

thanks,

greg k-h

2007-03-23 20:32:31

by Johannes Berg

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, 2007-03-23 at 21:27 +0100, Michael Buesch wrote:

> I finally have an USB ZD stick now, too. So I can test stuff, too.
> Should I test this somehow?

If you can, build libiw and wireless tools 32-bit programs and enable
the debug dump of the actual message (see one of my original mails),
then run them with scanning. You'll get a wrong message and some zeroes
where in my example the padding was not zeroed.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-03-23 20:07:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, 2007-03-23 at 13:00 -0700, Jean Tourrilhes wrote:

> Let's be more clear. I tested properly on 32 bits. But, 32
> bits is not where there is issue, this issue is specific to 64 bits. I
> don't have a 64 bits platfor. It seems that few people doing wireless
> have access to 64 bits platform (actually, it seems most of them don't
> have access to SMP either), otherwise this would have been found many
> years ago.
> Johannes is one of the few having access to 64 bits stuff.

Few, heh. I'll give it a try next week. (actually, I don't have wireless
on the 64-bit quad powermac but I can always plug in USB things)

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-03-23 20:03:32

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 12:36:43PM -0700, Chris Wright wrote:
> * Jean Tourrilhes ([email protected]) wrote:
>
> > The last thing is that I could not test those stuff myself,
> > and the testing done for me was limited. I would be more confident if
> > you were to give it a good beating before we push that into stable,
> > because you know what to test.
>
> That's useful info. Have you had any luck (it looks pretty straight
> forward)?

Let's be more clear. I tested properly on 32 bits. But, 32
bits is not where there is issue, this issue is specific to 64 bits. I
don't have a 64 bits platfor. It seems that few people doing wireless
have access to 64 bits platform (actually, it seems most of them don't
have access to SMP either), otherwise this would have been found many
years ago.
Johannes is one of the few having access to 64 bits stuff.

> thanks,
> -chris

Have fun...

Jean

2007-03-23 19:44:08

by Chris Wright

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

* Jean Tourrilhes ([email protected]) wrote:
> It seems that the lifetime of each stable branch is measured
> in weeks (apart from 2.6.16),

That's not accurate (apart from the fact that months are made of weeks ;-)

~2.5 months
2.6.15.1 Jan 15 2006
2.6.15.7 Mar 28 2006

[skipping 2.6.16 since it's still going]

~2 months
2.6.17.1 Aug 22 2006
2.6.17.14 Oct 13 2006

~3 months
2.6.18.1 Oct 14 2006
2.6.18.8 Feb 23 2007

~3 months
2.6.19.1 Dec 11 2006
2.6.19.7 Mar 3 2007

And to reiterate the sentiments of others. Security relevant bugs are
always of interest to the stable team as well as distros.

> The last thing is that I could not test those stuff myself,
> and the testing done for me was limited. I would be more confident if
> you were to give it a good beating before we push that into stable,
> because you know what to test.

That's useful info. Have you had any luck (it looks pretty straight
forward)?

thanks,
-chris

2007-03-23 20:54:59

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 09:27:17PM +0100, Michael Buesch wrote:
> On Friday 23 March 2007 21:00, Jean Tourrilhes wrote:
>
> I finally have an USB ZD stick now, too. So I can test stuff, too.
> Should I test this somehow?

Ok, here it goes.
Pick the latest version of wtools. You may want to compile it
static to avoid the need to install it.

To test the wpa patch, you need 32 bit userspace on 64 bit
kernel, and you need to try :
'iwlist genie'

To test the leak patch, you need a 64 kernel and any
userspace. You need two changes to enable the debugging code.
1) Put '#define DEBUG 1' on top of iwlib.c.
2) Change '#if 0' to '#if 1' line 783 of iwlist.c, at the
bottom of print_scanning_info().
Compile everything.
If you do a 'iwlist scan', it should dump the full content of
the stream. The first 4 bytes is the header (length + type). The next
4 bytes on 64 bits should be all zero.
This is what I don't want to see :
[19:00:1B:8B:50:8A:35:E0:09:00:01:00:50:8A:35:F0:47:6F:6C:6F:73:4E:65:74:7A
This is what I want to see :
[19:00:1B:8B:00:00:00:00:09:00:01:00:50:8A:35:F0:47:6F:6C:6F:73:4E:65:74:7A
Note that du to endian considerations you may have some of
your bytes swapped.

> Greetings Michael.

Good luck...

Jean

2007-03-26 15:48:51

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [stable] [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Sat, Mar 24, 2007 at 05:50:21PM +0100, Michael Buesch wrote:
> On Friday 23 March 2007 21:51, Jean Tourrilhes wrote:
> >
> > To test the leak patch, you need a 64 kernel and any
>
> Ok, so the most important question: Where to get the latest
> versions of these patches? :)
> I kind of lost track of what was agreed on and what's latest, etc..
> Some link to the mail?

I should send less e-mail to the mailing list so they are
easier to find :
http://marc.info/?l=linux-netdev&m=117460953806538&w=2
http://marc.info/?l=linux-netdev&m=117460988706771&w=2
You will find the same patch attached to this e-mail.

Jean


Attachments:
(No filename) (628.00 B)
iw261_wpa_compat_ioctl.diff (1.47 kB)
iw261_we22_64bit_usp_leak-3.diff (14.76 kB)
Download all attachments

2007-03-23 16:22:09

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 02:42:42PM +0100, Johannes Berg wrote:
> On Fri, 2007-03-23 at 14:40 +0100, Johannes Berg wrote:
> > Wireless extensions on 64-bit platforms leak information from the kernel
> > stack due to padding in structs that is copied. This affects any
> > wireless event stream including scan results and so hence is available
> > to unprivileged users.
>
> Ignore this patch, I see Jean just posted one as well :)

Hmmm...I seem to have missed it. Perhaps I was too quick w/ the 'd'
this morning? :-)

Jean, could you bounce me another copy?

Thanks,

John
--
John W. Linville
[email protected]

2007-03-23 16:16:00

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] fix information leak in wireless extensions on 64-bit platforms

On Fri, Mar 23, 2007 at 04:56:24PM +0100, Johannes Berg wrote:
> On Fri, 2007-03-23 at 08:48 -0700, Jean Tourrilhes wrote:
>
> > Yes, sorry for the delay, I wanted to make sure I had all the
> > parts fully fleshed out.
>
> :)
>
> Could you maybe send yours to stable too? Both of them I guess. Or do
> you want us (John I guess) to do that?
>
> johannes

It seems that the lifetime of each stable branch is measured
in weeks (apart from 2.6.16), and being an old timer, gaining a few
weeks does not seem to me to be worth the trouble. It's not like the
bug is entirely new.
I'm personally more concerned about distro. If getting it into
stable would make it go into a distro release, then I would see more
value. It seems that it's too late for the next release of Debian or
Fedora, and I don't know if any distro is going to be based on this
stable branch.
The last thing is that I could not test those stuff myself,
and the testing done for me was limited. I would be more confident if
you were to give it a good beating before we push that into stable,
because you know what to test.
Those are only my opinions. John is the man, so I let him
decide. John, if you feel like pushing it, please do it.

Have fun...

Jean