[PATCH] UBUNTU: SAUCE: ptrace: restrict ptrace scope to children

[Chase Douglas]

On Wed, 2010-05-12 at 15:22 -0700, Kees Cook wrote:
> As Linux grows in popularity, it will become a growing target for
> malware. One particularly troubling weakness of the Linux process
> interfaces is that a single user is able to examine the memory and
> running state of any of their processes. For example, if one application
> (e.g. Empathy) was compromised, it would be possible for an attacker to
> attach to other processes (e.g. Firefox) to extract additional credentials
> and continue to expand the scope of their attack.
> 
> For a solution, some applications use prctl() to specifically disallow
> such PTRACE attachment (e.g. ssh-agent). A more general solution is to
> only allow PTRACE directly from a parent to a child process (i.e. direct
> gdb and strace still work), or as the root user (i.e. gdb BIN PID,
> and strace -p PID still work as root).
> 
> This patch is based on the patch in grsecurity. I have added a sysctl to
> toggle the behavior back to the old scope via /proc/sys/kernel/ptrace_scope.
> 
> Signed-off-by: Kees Cook <kees.cook at canonical.com>
> ---
>  kernel/ptrace.c |   24 ++++++++++++++++++++++++
>  kernel/sysctl.c |   10 ++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 42ad8ae..ad80b43 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -24,6 +24,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/regset.h>
>  
> +/* sysctl for defining allowed scope of PTRACE */
> +int ptrace_scope = 1;
>  
>  /*
>   * ptrace a task: make the debugger its new parent and
> @@ -129,6 +131,10 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  	 * ptrace_attach denies several cases that /proc allows
>  	 * because setting up the necessary parent/child relationship
>  	 * or halting the specified task is impossible.
> +	 *
> +	 * PTRACE scope can be define as:
> +	 *  0 - classic: CAP_SYS_PTRACE and same uid can ptrace non-setuid
> +	 *  1 - restricted: as above, but only children of ptracing process
>  	 */
>  	int dumpable = 0;
>  	/* Don't let security modules deny introspection */
> @@ -152,6 +158,24 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
>  		dumpable = get_dumpable(task->mm);
>  	if (!dumpable && !capable(CAP_SYS_PTRACE))
>  		return -EPERM;
> +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> +		/* require ptrace target be a child of ptracer */
> +		struct task_struct *tmp = task;
> +		struct task_struct *curtemp = current;

Why create a new variable just to store current? I think it would be
more readable to just use current where you use curtemp. I don't think
current should change from under you when you're here.

> +		int rc = 0;
> +
> +		read_lock(&tasklist_lock);
> +		while (tmp->pid > 0) {
> +			if (tmp == curtemp)
> +				break;
> +			tmp = tmp->parent;
> +		}
> +		if (tmp->pid == 0)
> +			rc = -EPERM;
> +		read_unlock(&tasklist_lock);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	return security_ptrace_access_check(task, mode);
>  }