Re: More modutils: It's probably worse.

Re: More modutils: It's probably worse.

[Olaf Kirch]

On Tue, Nov 14, 2000 at 12:06:32AM +0100, Michal Zalewski wrote:
> Maybe I am missing something, but at least for me, modprobe
> vulnerabilities are exploitable via privledged networking services,
> nothing more.

Maybe not. ncpfs for instance has an ioctl that seems to allow
unprivileged users to specify a character set (codepage in m$speak)
that's requested via load_nls(), which in turn does a

	sprintf(buf, "nls_%s", codepage);
	request_module(buf);

Yummy.

The vfat file system contains code to obtain the charset name from
the media. Currently, the charset name is always "cp<number>", but
who knows, maybe next month will see another Microsoft extension to
ISO9660 that lets you specify an NLS name as a string?

Everyone is fixing modutils right now. Fine, but what about next
year's modutils rewrite?

This is why I keep repeating over and over again that we should make
sure request_module _does_not_ accept funky module names. Why allow
people to shoot themselves (and, by extension, all other Linux users
out there) in the foot?

Olaf

PS: The load_nls code tries to check for buffer overflows, but
    gets it wrong:

	struct nls_table *nls;
	char	buf[40];

	if (strlen(charset) > sizeof(buf) - sizeof("nls_"))
		fail;
	sprintf(buf, "nls_%s", charset);

    This will accept charset names of up to 35 characters,
    because sizeof("nls_") is 5. This gives you a single NUL byte
    overflow. Whether it's dangerous or not depends on whether your
    compiler reserves stack space for the *nls pointer or not...

-- 
Olaf Kirch         |  --- o --- Nous sommes du soleil we love when we play
okir@monad.swb.de  |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax
okir@caldera.de    +-------------------- Why Not?! -----------------------
         UNIX, n.: Spanish manufacturer of fire extinguishers.            
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[David Schleef]

On Tue, Nov 14, 2000 at 09:59:22AM +0100, Olaf Kirch wrote:
> On Tue, Nov 14, 2000 at 12:06:32AM +0100, Michal Zalewski wrote:
> > Maybe I am missing something, but at least for me, modprobe
> > vulnerabilities are exploitable via privledged networking services,
> > nothing more.
> 
> Maybe not. ncpfs for instance has an ioctl that seems to allow
> unprivileged users to specify a character set (codepage in m$speak)
> that's requested via load_nls(), which in turn does a
> 
> 	sprintf(buf, "nls_%s", codepage);
> 	request_module(buf);
> 
> Yummy.

Then it looks like the driver is broken, not modutils.


> Everyone is fixing modutils right now. Fine, but what about next
> year's modutils rewrite?
> 
> This is why I keep repeating over and over again that we should make
> sure request_module _does_not_ accept funky module names. Why allow
> people to shoot themselves (and, by extension, all other Linux users
> out there) in the foot?

Although I agree that having request_module() do a sanity check
is the best place to do a sanity check, I think it should be
up to the driver to not be stupid.  The drivers are trusted with
copy_to/from_user(), so why can't they be trusted to not pass
bad strings.

An inline function module_name_sanity_check() would be convenient
for those cases where "it is just necessary."

Rogue request_module() calls are bad in general, not only because
they might have dangerous invalid strings, but also because they
might have dangerous _valid_ strings.  I can imagine a
not-too-unlikely scenario where repeatedly loading a module
causes a DoS.




dave...


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Guest section DW]

On Tue, Nov 14, 2000 at 09:59:22AM +0100, Olaf Kirch wrote:

> PS: The load_nls code tries to check for buffer overflows, but
>     gets it wrong:
> 
> 	struct nls_table *nls;
> 	char	buf[40];
> 
> 	if (strlen(charset) > sizeof(buf) - sizeof("nls_"))
> 		fail;
> 	sprintf(buf, "nls_%s", charset);
> 
>     This will accept charset names of up to 35 characters,
>     because sizeof("nls_") is 5. This gives you a single NUL byte
>     overflow. Whether it's dangerous or not depends on whether your
>     compiler reserves stack space for the *nls pointer or not...

Where is the overflow? If charset has 35 characters then
	sprintf(buf, "nls_%s", charset);
writes 40 bytes into buf, and that fits.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Olaf Kirch]

On Tue, Nov 14, 2000 at 11:29:26AM +0100, Guest section DW wrote:
> Where is the overflow? If charset has 35 characters then
> 	sprintf(buf, "nls_%s", charset);
> writes 40 bytes into buf, and that fits.

Duh. You're right. Stupid me.

Sorry for the confusion.

Olaf
-- 
Olaf Kirch         |  --- o --- Nous sommes du soleil we love when we play
okir@monad.swb.de  |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax
okir@caldera.de    +-------------------- Why Not?! -----------------------
         UNIX, n.: Spanish manufacturer of fire extinguishers.            
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Petr Vandrovec]

On 14 Nov 00 at 2:04, David Schleef wrote:
> On Tue, Nov 14, 2000 at 09:59:22AM +0100, Olaf Kirch wrote:
> > On Tue, Nov 14, 2000 at 12:06:32AM +0100, Michal Zalewski wrote:
> > > Maybe I am missing something, but at least for me, modprobe
> > > vulnerabilities are exploitable via privledged networking services,
> > > nothing more.
> > 
> > Maybe not. ncpfs for instance has an ioctl that seems to allow
> > unprivileged users to specify a character set (codepage in m$speak)
> > that's requested via load_nls(), which in turn does a

> Then it looks like the driver is broken, not modutils.

Well, you can use this ioctl only before ncp filesystem gets to life,
but yes, as this call is always done by mount process, I'll add

if (!capable(CAP_SYS_ADMIN))
  return -EPERM;

here. But I still do not see any problem, as ncpfs limits charset/codepage
length to 20 chars (+ NUL terminator), and nobody told me that it is
not possible to use " or - in codepage name ;-)
                                                    Best regards,
                                                        Petr Vandrovec
                                                        vandrove@vc.cvut.cz
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Ben Ford]

Olaf Kirch wrote:

> sure request_module _does_not_ accept funky module names. Why allow
> people to shoot themselves (and, by extension, all other Linux users
> out there) in the foot?

I thought that was the whole purpose of Unix/Linux?

-b

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[H. Peter Anvin]

Followup to:  <20001114152430.C2645@alcove.wittsend.com>
By author:    "Michael H. Warfield" <mhw@wittsend.com>
In newsgroup: linux.dev.kernel
>
> 	Oh, I hate to add to a remark like that (OK, I lied, I love
> trollbait...)
> 
> On Tue, Nov 14, 2000 at 11:20:35AM -0800, Ben Ford wrote:
> > Olaf Kirch wrote:
> 
> > > sure request_module _does_not_ accept funky module names. Why allow
> > > people to shoot themselves (and, by extension, all other Linux users
> > > out there) in the foot?
> 
> > I thought that was the whole purpose of Unix/Linux?
> 
> 	True!  Very true!  Unix/Linux requires that the user shoot
> themselves in the foot.  Windows automates that process and does it
> for the user, thus making foot shooting user friendly.  :-)
> 

Seriously, though, I don't see any reason modprobe shouldn't accept
funky filenames.  There is a standard way to do that, which is to have
an argument consisting of the string "--"; this indicates that any
further arguments should be considered filenames and not options.

For example:

rm -- -foo		# Delete a file named "-foo"

	-hpa

-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
http://www.zytor.com/~hpa/puzzle.txt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Michael H. Warfield]

	Oh, I hate to add to a remark like that (OK, I lied, I love
trollbait...)

On Tue, Nov 14, 2000 at 11:20:35AM -0800, Ben Ford wrote:
> Olaf Kirch wrote:

> > sure request_module _does_not_ accept funky module names. Why allow
> > people to shoot themselves (and, by extension, all other Linux users
> > out there) in the foot?

> I thought that was the whole purpose of Unix/Linux?

	True!  Very true!  Unix/Linux requires that the user shoot
themselves in the foot.  Windows automates that process and does it
for the user, thus making foot shooting user friendly.  :-)

> -b

	Mike
-- 
 Michael H. Warfield    |  (770) 985-6132   |  mhw@WittsEnd.com
  (The Mad Wizard)      |  (678) 463-0932   |  http://www.wittsend.com/mhw/
  NIC whois:  MHW9      |  An optimist believes we live in the best of all
 PGP Key: 0xDF1DD471    |  possible worlds.  A pessimist is sure of it!

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Keith Owens]

On 14 Nov 2000 11:42:42 -0800, 
"H. Peter Anvin" <hpa@zytor.com> wrote:
>Seriously, though, I don't see any reason modprobe shouldn't accept
>funky filenames.  There is a standard way to do that, which is to have
>an argument consisting of the string "--"; this indicates that any
>further arguments should be considered filenames and not options.

The original exploit had nothing to do with filenames masquerading as
options, it was: ping6 -I ';chmod o+w .'.  Then somebody pointed out
that -I '-C/my/config/file' could be abused as well.  '--' fixes the
second exploit but not the first.

The problem is the combination of kernel code passing user space
parameters through unchanged (promoting user input to root) plus the
modprobe meta expansion algorithm.  By treating the last parameter from
the kernel as a tainted module name (not an option) and suppressing
meta expansion on tainted parameters, modprobe removes enough of the
problem to be safe.

My changes to modprobe do nothing about this: "ping6 -I binfmt_misc".
That construct lets a user load any module.  However that is a pure
kernel problem which needs to be fixed by the developers who call
request_module.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Olaf Titz]

> The original exploit had nothing to do with filenames masquerading as
> options, it was: ping6 -I ';chmod o+w .'.  Then somebody pointed out

Why is there any reason that a shell should be invoked anywhere in the
request_module->modprobe->insmod chain?
If implemented correctly, this attack should have the same result as
insmod ';chmod o+w .' (and it should not matter if it gets renamed so
that the actual command executed is insmod 'netdevice-;chmod o+w .')

> The problem is the combination of kernel code passing user space
> parameters through unchanged (promoting user input to root)

Which means that all parts of the chain which deal with possible user
input in elevated privilege mode must do input validation. This means
the kernel _and_ modprobe in my book.

> plus the
> modprobe meta expansion algorithm.

and I see no reason why modprobe should do any such thing, apart from
configurations dealt with in modules.conf anyway.

Olaf
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Tim Waugh]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On Wed, Nov 15, 2000 at 11:43:54AM +0100, Olaf Titz wrote:

> > plus the
> > modprobe meta expansion algorithm.
> 
> and I see no reason why modprobe should do any such thing, apart from
> configurations dealt with in modules.conf anyway.

If it helps, wordexp has a flag to prevent command substitutions from
occuring.

Tim.
*/

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: More modutils: It's probably worse.

[Keith Owens]

On Wed, 15 Nov 2000 11:43:54 +0100, 
>Why is there any reason that a shell should be invoked anywhere in the
>request_module->modprobe->insmod chain?
>If implemented correctly, this attack should have the same result as
>insmod ';chmod o+w .' (and it should not matter if it gets renamed so
>that the actual command executed is insmod 'netdevice-;chmod o+w .')

You are confusing two different problems.  The meta expansion problem
means ;chmod o+w .' does nasty things to your system.  The other
problem is that any user can load any module by ping6 -I module_name.

>> plus the
>> modprobe meta expansion algorithm.
>
>and I see no reason why modprobe should do any such thing, apart from
>configurations dealt with in modules.conf anyway.

modutils 2.3.20 only does meta expansion for entries in the config
file, not for input from the command line.  That fixes the first
problem but does nothing about the second.  The only way to fix the
second problem is by always adding a prefix to the user input before
passing it to modprobe, that fix has to be in the kernel, not modutils.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: More modutils: It's probably worse.

[Rusty Russell]

In message <11900.974244463@ocs3.ocs-net> you write:
> On 14 Nov 2000 11:42:42 -0800, 
> "H. Peter Anvin" <hpa@zytor.com> wrote:
> >Seriously, though, I don't see any reason modprobe shouldn't accept
> >funky filenames.  There is a standard way to do that, which is to have
> >an argument consisting of the string "--"; this indicates that any
> >further arguments should be considered filenames and not options.
> 
> The original exploit had nothing to do with filenames masquerading as
> options, it was: ping6 -I ';chmod o+w .'.  Then somebody pointed out
> that -I '-C/my/config/file' could be abused as well.  '--' fixes the
> second exploit but not the first.

Yes, modprobe code is stupid (execing insmod without "--").  Of
course, the passing of flags to modprobe is pretty broken too (the
kernel shouldn't assume anything about modprobe, otherwise why bother
with the /proc entry?)

But the kernel should be fixed for future:

--- working-2.4.0-test11-5/kernel/kmod.c.~1~	Wed Oct  4 15:17:12 2000
+++ working-2.4.0-test11-5/kernel/kmod.c	Fri Nov 17 11:44:09 2000
@@ -133,7 +133,7 @@
 static int exec_modprobe(void * module_name)
 {
 	static char * envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { modprobe_path, "-s", "-k", (char*)module_name, NULL };
+	char *argv[] = { modprobe_path, "-s", "-k", "--", (char*)module_name, NULL };
 	int ret;
 
 	ret = exec_usermodehelper(modprobe_path, argv, envp);
--
Hacking time.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/