[PATCH] slim: fix security issue with the task_post_setuid hook

From: Kylene Jo Hall
Date: Mon Sep 18 2006 - 14:10:32 EST


Much thanks to Stephen Smalley for finding this security hole opened by
not calling the dummy_ops function in the task_post_setuid hook of the
SLIM LSM. This patch fixes that as well as resolves an existing issue
where we should have been handling all LSM_SETID types.

Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxx>
Signed-off-by: Kylene Hall <kjhall@xxxxxxxxxx>
---
security/slim/slm_main.c | 77 ++++++++++++++++++++-----------------
1 files changed, 42 insertions(+), 35 deletions(-)

Index: linux-2.6.18-rc6-mm2/security/slim/slm_main.c
===================================================================
--- linux-2.6.18-rc6-mm2.orig/security/slim/slm_main.c
+++ linux-2.6.18-rc6-mm2/security/slim/slm_main.c
@@ -29,6 +29,8 @@

#include "slim.h"

+extern struct security_operations dummy_security_ops;
+
unsigned int slm_debug = SLM_BASE;
#define XATTR_NAME "security.slim.level"

@@ -1196,43 +1199,48 @@ static int slm_task_post_setuid(uid_t ol
uid_t old_suid, int flags)
{
struct slm_tsec_data *cur_tsec = current->security;
+ int rc;

- if (cur_tsec && flags == LSM_SETID_ID) {
- /*set process to USER level integrity for everything but root */
- dprintk(SLM_VERBOSE, "ruid %d euid %d suid %d "
- "cur: uid %d euid %d suid %d\n",
+ /*set process to USER level integrity for everything but root */
+ dprintk(SLM_VERBOSE, "ruid %d euid %d suid %d "
+ "cur: uid %d euid %d suid %d "
+ "permitted %x effective %x\n",
old_ruid, old_euid, old_suid,
- current->uid, current->euid, current->suid);
- spin_lock(&cur_tsec->lock);
- if ((cur_tsec->iac_r == cur_tsec->iac_wx)
- && (cur_tsec->iac_r == SLM_IAC_UNTRUSTED)) {
- dprintk(SLM_INTEGRITY,
- "Integrity: pid %d iac_r %d "
- " iac_wx %d remains UNTRUSTED\n",
- current->pid, cur_tsec->iac_r,
- cur_tsec->iac_wx);
- } else if (current->suid != 0) {
- dprintk(SLM_INTEGRITY, "setting: pid %d iac_r %d "
- " iac_wx %d to USER\n",
- current->pid, cur_tsec->iac_r,
- cur_tsec->iac_wx);
- cur_tsec->iac_r = SLM_IAC_USER;
- cur_tsec->iac_wx = SLM_IAC_USER;
- } else if ((current->uid == 0) && (old_ruid != 0)) {
- dprintk(SLM_INTEGRITY, "setting: pid %d iac_r %d "
- " iac_wx %d to SYSTEM\n",
- current->pid, cur_tsec->iac_r,
- cur_tsec->iac_wx);
- cur_tsec->iac_r = SLM_IAC_SYSTEM;
- cur_tsec->iac_wx = SLM_IAC_SYSTEM;
- } else
- dprintk(SLM_INTEGRITY, "%s: pid %d iac_r %d "
- " iac_wx %d \n", __FUNCTION__,
- current->pid, cur_tsec->iac_r,
- cur_tsec->iac_wx);
- spin_unlock(&cur_tsec->lock);
- }
- return 0;
+ current->uid, current->euid, current->suid,
+ current->cap_permitted, current->cap_effective);
+ rc = dummy_security_ops.task_post_setuid(old_ruid, old_euid,
+ old_suid, flags);
+ spin_lock(&cur_tsec->lock);
+ if ((cur_tsec->iac_r == cur_tsec->iac_wx)
+ && (cur_tsec->iac_r == SLM_IAC_UNTRUSTED)) {
+ dprintk(SLM_INTEGRITY,
+ "Integrity: pid %d iac_r %d "
+ " iac_wx %d remains UNTRUSTED\n",
+ current->pid, cur_tsec->iac_r,
+ cur_tsec->iac_wx);
+ current->cap_permitted = 0;
+ current->cap_effective = 0;
+ } else if (current->suid != 0) {
+ dprintk(SLM_INTEGRITY, "setting: pid %d iac_r %d "
+ " iac_wx %d to USER\n",
+ current->pid, cur_tsec->iac_r,
+ cur_tsec->iac_wx);
+ cur_tsec->iac_r = SLM_IAC_USER;
+ cur_tsec->iac_wx = SLM_IAC_USER;
+ } else if ((current->uid == 0) && (old_ruid != 0)) {
+ dprintk(SLM_INTEGRITY, "setting: pid %d iac_r %d "
+ " iac_wx %d to SYSTEM\n",
+ current->pid, cur_tsec->iac_r,
+ cur_tsec->iac_wx);
+ cur_tsec->iac_r = SLM_IAC_SYSTEM;
+ cur_tsec->iac_wx = SLM_IAC_SYSTEM;
+ } else
+ dprintk(SLM_INTEGRITY, "%s: pid %d iac_r %d "
+ " iac_wx %d \n", __FUNCTION__,
+ current->pid, cur_tsec->iac_r,
+ cur_tsec->iac_wx);
+ spin_unlock(&cur_tsec->lock);
+ return rc;
}

static inline int slm_setprocattr(struct task_struct *tsk,


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