From ea579a477de5aabed2fd91f73b46be6afbfb2f6d Mon Sep 17 00:00:00 2001 From: sef Date: Tue, 12 Aug 1997 04:34:30 +0000 Subject: [PATCH] Fix procfs security hole -- check permissions on meaningful I/Os (namely, reading/writing of mem and regs). Also have to check for the requesting process being group KMEM -- this is a bit of a hack, but ps et al need it. Reviewed by: davidg --- sys/fs/procfs/procfs.h | 14 +++++++++++++- sys/fs/procfs/procfs_mem.c | 19 ++++++++++++++++++- sys/fs/procfs/procfs_regs.c | 4 +++- sys/fs/procfs/procfs_vnops.c | 14 +++++++++----- sys/miscfs/procfs/procfs.h | 14 +++++++++++++- sys/miscfs/procfs/procfs_mem.c | 19 ++++++++++++++++++- sys/miscfs/procfs/procfs_regs.c | 4 +++- sys/miscfs/procfs/procfs_vnops.c | 14 +++++++++----- 8 files changed, 86 insertions(+), 16 deletions(-) diff --git a/sys/fs/procfs/procfs.h b/sys/fs/procfs/procfs.h index 2821aa268a6..9e051d49b1e 100644 --- a/sys/fs/procfs/procfs.h +++ b/sys/fs/procfs/procfs.h @@ -37,7 +37,7 @@ * @(#)procfs.h 8.9 (Berkeley) 5/14/95 * * From: - * $Id$ + * $Id: procfs.h,v 1.15 1997/02/22 09:40:26 peter Exp $ */ /* @@ -85,6 +85,18 @@ struct pfsnode { (bcmp((s), (cnp)->cn_nameptr, (len)) == 0)) #define KMEM_GROUP 2 + +/* + * Check to see whether access to target process is allowed + * Evaluates to 1 if access is allowed. + */ +#define CHECKIO(p1, p2) \ + ((((p1)->p_cred->pc_ucred->cr_uid == (p2)->p_cred->p_ruid) && \ + ((p1)->p_cred->p_ruid == (p2)->p_cred->p_ruid) && \ + ((p1)->p_cred->p_svuid == (p2)->p_cred->p_ruid) && \ + ((p2)->p_flag & P_SUGID) == 0) || \ + (suser((p1)->p_cred->pc_ucred, &(p1)->p_acflag) == 0)) + /* * Format of a directory entry in /proc, ... * This must map onto struct dirent (see ) diff --git a/sys/fs/procfs/procfs_mem.c b/sys/fs/procfs/procfs_mem.c index 97b7d9b584b..1a9d6aba659 100644 --- a/sys/fs/procfs/procfs_mem.c +++ b/sys/fs/procfs/procfs_mem.c @@ -37,7 +37,7 @@ * * @(#)procfs_mem.c 8.5 (Berkeley) 6/15/94 * - * $Id: procfs_mem.c,v 1.25 1997/04/20 17:12:11 dyson Exp $ + * $Id: procfs_mem.c,v 1.26 1997/08/02 14:32:14 bde Exp $ */ /* @@ -277,6 +277,23 @@ procfs_domem(curp, p, pfs, uio) if (uio->uio_resid == 0) return (0); + /* + * XXX + * We need to check for KMEM_GROUP because ps is sgid kmem; + * not allowing it here causes ps to not work properly. Arguably, + * this is a bug with what ps does. We only need to do this + * for Pmem nodes, and only if it's reading. This is still not + * good, as it may still be possible to grab illicit data if + * a process somehow gets to be KMEM_GROUP. Note that this also + * means that KMEM_GROUP can't change without editing procfs.h! + * All in all, quite yucky. + */ + + if (!CHECKIO(curp, p) && + !(curp->p_cred->pc_ucred->cr_gid == KMEM_GROUP && + uio->uio_rw == UIO_READ)) + return EPERM; + return (procfs_rwmem(p, uio)); } diff --git a/sys/fs/procfs/procfs_regs.c b/sys/fs/procfs/procfs_regs.c index 276c5ed6220..d215d44a33c 100644 --- a/sys/fs/procfs/procfs_regs.c +++ b/sys/fs/procfs/procfs_regs.c @@ -37,7 +37,7 @@ * @(#)procfs_regs.c 8.4 (Berkeley) 6/15/94 * * From: - * $Id: procfs_regs.c,v 1.6 1997/02/22 09:40:29 peter Exp $ + * $Id: procfs_regs.c,v 1.7 1997/08/02 14:32:16 bde Exp $ */ #include @@ -60,6 +60,8 @@ procfs_doregs(curp, p, pfs, uio) char *kv; int kl; + if (!CHECKIO(curp, p)) + return EPERM; kl = sizeof(r); kv = (char *) &r; diff --git a/sys/fs/procfs/procfs_vnops.c b/sys/fs/procfs/procfs_vnops.c index 77f2e49cc85..f876318e6ae 100644 --- a/sys/fs/procfs/procfs_vnops.c +++ b/sys/fs/procfs/procfs_vnops.c @@ -36,7 +36,7 @@ * * @(#)procfs_vnops.c 8.18 (Berkeley) 5/21/95 * - * $Id: procfs_vnops.c,v 1.29 1997/02/24 16:44:11 bde Exp $ + * $Id: procfs_vnops.c,v 1.30 1997/08/02 14:32:20 bde Exp $ */ /* @@ -127,16 +127,21 @@ procfs_open(ap) } */ *ap; { struct pfsnode *pfs = VTOPFS(ap->a_vp); + struct proc *p1 = ap->a_p, *p2 = PFIND(pfs->pfs_pid); + + if (p2 == NULL) + return ENOENT; switch (pfs->pfs_type) { case Pmem: - if (PFIND(pfs->pfs_pid) == 0) - return (ENOENT); /* was ESRCH, jsp */ - if ((pfs->pfs_flags & FWRITE) && (ap->a_mode & O_EXCL) || (pfs->pfs_flags & O_EXCL) && (ap->a_mode & FWRITE)) return (EBUSY); + if (!CHECKIO(p1, p2) && + (p1->p_cred->pc_ucred->cr_gid != KMEM_GROUP)) + return EPERM; + if (ap->a_mode & FWRITE) pfs->pfs_flags = ap->a_mode & (FWRITE|O_EXCL); @@ -194,7 +199,6 @@ procfs_ioctl(ap) struct proc *a_p; } */ *ap; { - return (ENOTTY); } diff --git a/sys/miscfs/procfs/procfs.h b/sys/miscfs/procfs/procfs.h index 2821aa268a6..9e051d49b1e 100644 --- a/sys/miscfs/procfs/procfs.h +++ b/sys/miscfs/procfs/procfs.h @@ -37,7 +37,7 @@ * @(#)procfs.h 8.9 (Berkeley) 5/14/95 * * From: - * $Id$ + * $Id: procfs.h,v 1.15 1997/02/22 09:40:26 peter Exp $ */ /* @@ -85,6 +85,18 @@ struct pfsnode { (bcmp((s), (cnp)->cn_nameptr, (len)) == 0)) #define KMEM_GROUP 2 + +/* + * Check to see whether access to target process is allowed + * Evaluates to 1 if access is allowed. + */ +#define CHECKIO(p1, p2) \ + ((((p1)->p_cred->pc_ucred->cr_uid == (p2)->p_cred->p_ruid) && \ + ((p1)->p_cred->p_ruid == (p2)->p_cred->p_ruid) && \ + ((p1)->p_cred->p_svuid == (p2)->p_cred->p_ruid) && \ + ((p2)->p_flag & P_SUGID) == 0) || \ + (suser((p1)->p_cred->pc_ucred, &(p1)->p_acflag) == 0)) + /* * Format of a directory entry in /proc, ... * This must map onto struct dirent (see ) diff --git a/sys/miscfs/procfs/procfs_mem.c b/sys/miscfs/procfs/procfs_mem.c index 97b7d9b584b..1a9d6aba659 100644 --- a/sys/miscfs/procfs/procfs_mem.c +++ b/sys/miscfs/procfs/procfs_mem.c @@ -37,7 +37,7 @@ * * @(#)procfs_mem.c 8.5 (Berkeley) 6/15/94 * - * $Id: procfs_mem.c,v 1.25 1997/04/20 17:12:11 dyson Exp $ + * $Id: procfs_mem.c,v 1.26 1997/08/02 14:32:14 bde Exp $ */ /* @@ -277,6 +277,23 @@ procfs_domem(curp, p, pfs, uio) if (uio->uio_resid == 0) return (0); + /* + * XXX + * We need to check for KMEM_GROUP because ps is sgid kmem; + * not allowing it here causes ps to not work properly. Arguably, + * this is a bug with what ps does. We only need to do this + * for Pmem nodes, and only if it's reading. This is still not + * good, as it may still be possible to grab illicit data if + * a process somehow gets to be KMEM_GROUP. Note that this also + * means that KMEM_GROUP can't change without editing procfs.h! + * All in all, quite yucky. + */ + + if (!CHECKIO(curp, p) && + !(curp->p_cred->pc_ucred->cr_gid == KMEM_GROUP && + uio->uio_rw == UIO_READ)) + return EPERM; + return (procfs_rwmem(p, uio)); } diff --git a/sys/miscfs/procfs/procfs_regs.c b/sys/miscfs/procfs/procfs_regs.c index 276c5ed6220..d215d44a33c 100644 --- a/sys/miscfs/procfs/procfs_regs.c +++ b/sys/miscfs/procfs/procfs_regs.c @@ -37,7 +37,7 @@ * @(#)procfs_regs.c 8.4 (Berkeley) 6/15/94 * * From: - * $Id: procfs_regs.c,v 1.6 1997/02/22 09:40:29 peter Exp $ + * $Id: procfs_regs.c,v 1.7 1997/08/02 14:32:16 bde Exp $ */ #include @@ -60,6 +60,8 @@ procfs_doregs(curp, p, pfs, uio) char *kv; int kl; + if (!CHECKIO(curp, p)) + return EPERM; kl = sizeof(r); kv = (char *) &r; diff --git a/sys/miscfs/procfs/procfs_vnops.c b/sys/miscfs/procfs/procfs_vnops.c index 77f2e49cc85..f876318e6ae 100644 --- a/sys/miscfs/procfs/procfs_vnops.c +++ b/sys/miscfs/procfs/procfs_vnops.c @@ -36,7 +36,7 @@ * * @(#)procfs_vnops.c 8.18 (Berkeley) 5/21/95 * - * $Id: procfs_vnops.c,v 1.29 1997/02/24 16:44:11 bde Exp $ + * $Id: procfs_vnops.c,v 1.30 1997/08/02 14:32:20 bde Exp $ */ /* @@ -127,16 +127,21 @@ procfs_open(ap) } */ *ap; { struct pfsnode *pfs = VTOPFS(ap->a_vp); + struct proc *p1 = ap->a_p, *p2 = PFIND(pfs->pfs_pid); + + if (p2 == NULL) + return ENOENT; switch (pfs->pfs_type) { case Pmem: - if (PFIND(pfs->pfs_pid) == 0) - return (ENOENT); /* was ESRCH, jsp */ - if ((pfs->pfs_flags & FWRITE) && (ap->a_mode & O_EXCL) || (pfs->pfs_flags & O_EXCL) && (ap->a_mode & FWRITE)) return (EBUSY); + if (!CHECKIO(p1, p2) && + (p1->p_cred->pc_ucred->cr_gid != KMEM_GROUP)) + return EPERM; + if (ap->a_mode & FWRITE) pfs->pfs_flags = ap->a_mode & (FWRITE|O_EXCL); @@ -194,7 +199,6 @@ procfs_ioctl(ap) struct proc *a_p; } */ *ap; { - return (ENOTTY); } -- 2.45.2