Local root exploit with kmod and modutils > 2.1.121

Local root exploit with kmod and modutils > 2.1.121

[Keith Owens]

On Mon, 13 Nov 2000 20:23:07 +0000 (GMT), 
Chris Evans <chris@scary.beasts.org> wrote:
>Either the kernel or modprobe needs to treat the module name with
>_extreme_ distrust, and I see no evidence of that.

Agreed.  modprobe was not designed to run suid.  Calling modprobe from
request_module has the same effect as running suid.  dev_load() can
take the interface name and pass it to modprobe unchanged and modprobe
does not verify its input, it trusts root/kernel.

>Without this distrust, there is a huge amount of code a malicious user can
>play with. Looking at meta_expand() and split_line() in modprobe, there
>are buffer overflows all over the place (lucky an interface name can only
>be 15 characters!). Worse, user-defined input can be passed to sprawling
>interfaces such as glob() and wordexp().

The buffer overflows are not as bad as they look, most of them are on
malloc strings which have calculated sizes.  There are some strcat
calls that are suspect and I have fixed those in my tree, and replaced
the system() calls with safe equivalents.  But that does not fix the
other problems.

(1) Some user defined input is passed directly through the kernel to
    modprobe running as root.

(2) Current modprobe has no way of telling that it was invoked from the
    kernel instead of by root so it cannot apply different verification
    to its parameters for kmod input.

(3) modprobe applies filename expansion to the user supplied input as
    well as the paths from modules.conf.  The former is tainted, the
    latter is not but they are joined together in modprobe.  This is
    fine for root, terrible for kmod.

The only secure fix I can see is to add SAFEMODE=1 to modprobe's
environment and change exec_modprobe.

static char * envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", "SAFEMODE=1", NULL };

In safemode, modprobe will treat its last argument as a module name,
even if it starts with '-'.  Also in safemode, no filename expansion
will be applied to the module name, filename expansion will still be
applied to paths in /etc/modules.conf.

To cater for older kernels (including 2.2), modprobe will treat an
environment of exactly this, and no more
"HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin"
as requiring safemode.  Why add SAFEMODE at all?  In case we want to
change the environment in future.

Chris Evans raised another point.

(4) Passing user defined input directly through the kernel to modprobe
    does more than expose modprobe to suspect input.  If a user can
    find a program that the kernel trusts and whose input is passed
    straight through then they can load any module.  Controlled loading
    with kernel generated names like net-pf-10, eth0 is fine,
    uncontrolled loading of any module name from user input is not.
    This is a pure kernel problem.

-
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: Local root exploit with kmod and modutils > 2.1.121

[Alan Cox]

> request_module has the same effect as running suid.  dev_load() can
> take the interface name and pass it to modprobe unchanged and modprobe
> does not verify its input, it trusts root/kernel.

Then dev_load is being called the wrong way. In older kernels we explicitly
only did a dev_load with user passed names providing suser() was true.

Alan

-
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: Local root exploit with kmod and modutils > 2.1.121

[kuznet]

Hello!

> > request_module has the same effect as running suid.  dev_load() can
> > take the interface name and pass it to modprobe unchanged and modprobe
> > does not verify its input, it trusts root/kernel.
> 
> Then dev_load is being called the wrong way. In older kernels we explicitly
> only did a dev_load with user passed names providing suser() was true.

It checks CAP_SYS_MODULE nowadays.

Which does not look good by the way, it is function of request_module(),
rather than of caller.

Alexey
-
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: Local root exploit with kmod and modutils > 2.1.121

[Alan Cox]

> It checks CAP_SYS_MODULE nowadays.
> 
> Which does not look good by the way, it is function of request_module(),
> rather than of caller.

Only the caller knows if the data is tainted. Thus only the caller can decide

-
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: Local root exploit with kmod and modutils > 2.1.121

[kuznet]

Hello!

> Only the caller knows if the data is tainted. Thus only the caller can decide

Sorry? What data? What to decide?

Device name of &|&|&|&|&|& is absolutely legal, nicely loking name.
dev.c _wants_ to load such device and it is problem of kmod,
if it is not able to make this.

It is the first. And the second: each user is allowed to refer to this device.
And it is problem of module to allow to load corresponding module or not
to allow this.

It means that test for CAP_SYS_MODULE is illegal, moving pure policy
issue to improper place.

Alexey
-
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: Local root exploit with kmod and modutils > 2.1.121

[Alan Cox]

> It is the first. And the second: each user is allowed to refer to this device.
> And it is problem of module to allow to load corresponding module or not
> to allow this.

Not so.

> It means that test for CAP_SYS_MODULE is illegal, moving pure policy
> issue to improper place.

Definitely not so

What matters is whether the user is requesting a module or the kernel is 
choosing to load a module. In the former case where the user can influence the
module name then you need to check CAP_SYS_MODULE in the latter you do not
care.

Alan
-
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: Local root exploit with kmod and modutils > 2.1.121

[kuznet]

Hello!

> > It means that test for CAP_SYS_MODULE is illegal, moving pure policy
> > issue to improper place.
> 
> Definitely not so
> 
> What matters is whether the user is requesting a module or the kernel is 
> choosing to load a module. In the former case where the user can influence the
> module name then you need to check CAP_SYS_MODULE in the latter you do not
> care.

Alan, I honestly peered to this paragraph of text for 10 minutes. 8)8)

It is funny, but I managed to compile it only as:
"dev_load(i.e. you) need not take of care of this".

I.e. exactly the thing which I said. 8)

Alexey
-
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/

[PATCH] Re: Local root exploit with kmod and modutils > 2.1.121

[Xavier Bestel]

Hi,

as modprobe (insmod) seems to have POSIX args handling, we should perhaps add "--"
to the modprobe cmdline, in order to stop further args processing, and to avoid
mixing a textual argument with an option.
BTW, it should perhaps be generalized.

Xav


--- linux-2.4-test10/kernel/kmod.c	Tue Sep 26 01:18:55 2000
+++ linux/kernel/kmod.c	Thu Nov 16 19:57:45 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);


-
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: Local root exploit with kmod and modutils > 2.1.121

[Keith Owens]

On Thu, 16 Nov 2000 16:04:23 +0000 (GMT), 
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> request_module has the same effect as running suid.  dev_load() can
>> take the interface name and pass it to modprobe unchanged and modprobe
>> does not verify its input, it trusts root/kernel.
>
>Then dev_load is being called the wrong way. In older kernels we explicitly
>only did a dev_load with user passed names providing suser() was true.

ping6 -I module_name.  ping6 is setuid, it passes the interface name to
the kernel while it holds root privileges, suser() == true.  It is
not reasonable to expect setuid programs to know that Linux does
something special with some parameters when no other O/S has that
"feature".

-
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: Local root exploit with kmod and modutils > 2.1.121

[Alan Cox]

> >Then dev_load is being called the wrong way. In older kernels we explicitly
> >only did a dev_load with user passed names providing suser() was true.
> 
> ping6 -I module_name.  ping6 is setuid, it passes the interface name to
> the kernel while it holds root privileges, suser() == true.  It is
> not reasonable to expect setuid programs to know that Linux does
> something special with some parameters when no other O/S has that
> "feature".

ping6 shouldnt be running with CAP_SYS_MODULE in the first place

-
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: Local root exploit with kmod and modutils > 2.1.121

[Adam J. Richter]

>The only secure fix I can see is to add SAFEMODE=1 to modprobe's
>environment and change exec_modprobe.

	SAFEMODE may mean other things to other programs, so that
an ordinary user might set that environment variable for some
other reason, and then get weird behavior if he or she has occasion
to su to root.  In general, you only want to use environment variables
if either it is a user interface issue to keep the commands short
(not an issue here, since nobody is typing in the command that
requrest_module generates) or there is some well established
convention that will handled in a particular way by subordinate
child processes (e.g., PATH=....).

	It would be much better to just add a command line option
to modprobe that request_module() would cause it treat the following
argument as the module to load (you do not ever have to force
argument processing to stop at that point, since module will be
fully contained in the next argument, even if it contains space or
linefeed).

	Another possible approach would be to create a separate
/sbin/safe_modprobe.  modprobe already behaves differently
based on whether argv[0] ends in "modprobe", "insmod", "depmod",
or "rmmod".  So this would be in keeping with that convention.
It would also be trivial to retrofit old systems.  Just have
some system boot script do:

		echo /sbin/safe_modprobe > /proc/sys/kernel/modprobe

	The issue of the kernel doing request_module() on arbitrary
strings is not just a security problem.  It is also a namespace
collision problem, which this security concern will give us the
opportunity to fix.  I have just been glad that no company has
shipped a networking device called, say, "ext2".  The non-constant
module names that are loaded by request_module should have names like:

		fs-msdos
		fs-ext2
		netif-eth0
		netif-wvlan0
		etc.

	That way, a malicious user cannot cause a denial of service
by identifying one module with a loading bug (our kernels have 774 modules)
and doing, "ifconfig <modulename>".

	The extra work of doing the snprintf() into a buffer
before invoking request_module will resolve the buffer overrun
issues too.

	I would be happy to assist in coding this up.  The 50 lines of
text that I have written in this email probably only translate into
20 lines of code.

Adam J. Richter     __     ______________   4880 Stevens Creek Blvd, Suite 104
adam@yggdrasil.com     \ /                  San Jose, California 95129-1034
+1 408 261-6630         | g g d r a s i l   United States of America
fax +1 408 261-6631      "Free Software For The Rest Of Us."
-
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: Local root exploit with kmod and modutils > 2.1.121

[Keith Owens]

On Tue, 14 Nov 2000 12:31:41 -0800, 
"Adam J. Richter" <adam@freya.yggdrasil.com> wrote:
>>The only secure fix I can see is to add SAFEMODE=1 to modprobe's
>>environment and change exec_modprobe.
>
>	SAFEMODE may mean other things to other programs, so that

MOD_SAFEMODE.

>	It would be much better to just add a command line option
>to modprobe that request_module() would cause it treat the following

Changing the command line is not an option.  Kernel 2.2 still runs with
modutils 2.1.121, changing the request_module command line would break
people using modutils 2.1.121 and force them to upgrade, AC would kill
me.  I needed a mechanism that would work with modutils 2.3 but have no
effect on modutils 2.1.121, remember that 2.1.121 does not have this
security exposure.  It also had to work on 2.2 kernels because many
people are using moditils 2.3 on 2.2 kernels.  SGI ship a 2.2 kernel
with devfs for their big machines, that needs modutils 2.3.

>	Another possible approach would be to create a separate
>/sbin/safe_modprobe.  modprobe already behaves differently
>based on whether argv[0] ends in "modprobe", "insmod", "depmod",
>or "rmmod".  So this would be in keeping with that convention.
>It would also be trivial to retrofit old systems.  Just have
>some system boot script do:
>
>		echo /sbin/safe_modprobe > /proc/sys/kernel/modprobe

I thought about that but it assumes that users will add that line to
their scripts - not guaranteed.  The fix needed a change that would
automatically detect that safe mode was required and not rely on manual
intervention.  Especially with 30+ Linux distributions out there.

-
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/

Local root exploit with kmod and modutils > 2.1.121

[Keith Owens]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Content-Type: text/plain; charset=us-ascii

A local root exploit has been found using kernels compiled with kmod
and modutils > 2.1.121.  Kernels without kmod and systems using
modutils 2.1.121 are not affected.

Patch against modutils 2.3.19, it should fit any 2.3 modutils.

Index: 19.7/util/meta_expand.c
- --- 19.7/util/meta_expand.c Sun, 10 Sep 2000 12:56:40 +1100 kaos (modutils-2.3/10_meta_expan 1.4 644)
+++ 19.7(w)/util/meta_expand.c Mon, 13 Nov 2000 21:19:41 +1100 kaos (modutils-2.3/10_meta_expan 1.4 644)
@@ -156,12 +156,8 @@ static int glob_it(char *pt, GLOB_LIST *
  */
 int meta_expand(char *pt, GLOB_LIST *g, char *base_dir, char *version)
 {
- -	FILE *fin;
- -	int len = 0;
- -	char *line = NULL;
 	char *p;
 	char tmpline[PATH_MAX + 1];
- -	char tmpcmd[PATH_MAX + 11];
 
 	g->pathc = 0;
 	g->pathv = NULL;
@@ -277,38 +273,6 @@ int meta_expand(char *pt, GLOB_LIST *g, 
 		/* Only "=" remaining, should be module options */
 		split_line(g, pt, 0);
 		return 0;
- -	}
- -
- -	/*
- -	 * Last resort: Use "echo"
- -	 */
- -	sprintf(tmpline, "%s%s", (base_dir ? base_dir : ""), pt);
- -	sprintf(tmpcmd, "/bin/echo %s", tmpline);
- -	if ((fin = popen(tmpcmd, "r")) == NULL) {
- -		error("Can't execute: %s", tmpcmd);
- -		return -1;
- -	}
- -	/* else */
- -
- -	/*
- -	 * Collect the result
- -	 */
- -	while (fgets(tmpcmd, PATH_MAX, fin) != NULL) {
- -		int l = strlen(tmpcmd);
- -
- -		line = (char *)xrealloc(line, len + l + 1);
- -		line[len] = '\0';
- -		strcat(line + len, tmpcmd);
- -		len += l;
- -	}
- -	pclose(fin);
- -
- -	if (line) {
- -		/* Ignore result if no expansion occurred */
- -		strcat(tmpline, "\n");
- -		if (strcmp(tmpline, line))
- -			split_line(g, line, 0);
- -		free(line);
 	}
 
 	return 0;

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.3 (GNU/Linux)
Comment: Exmh version 2.1.1 10/15/1999

iD8DBQE6D8kEi4UHNye0ZOoRAmVTAKCktbi9DI5t0sj8wd1/vjLtgwVW6QCgnO0L
mVbPskoIGSSyTE8I9K7FcAg=
=Z1/L
-----END PGP SIGNATURE-----

-
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/