SECURITY: Unbreak “set +p”, broken by OpenBSD ksh change.

TODO: I am seriously considering following Chet and changing
the way this works, by explicitly dropping privs unless the
shell is run with -p. Every other shell does it like mksh,
except Heirloom sh, which on the other hand doesn’t know any
explicit set -p or set +p (though it doesn’t know set +foo
for any foo either).

┌──┤ QUESTION: Do we need the ability to do this:
│ tg@blau:~ $ ./suidmksh -p -c 'whoami; set +p; whoami'
│ root
│ tg

If not, I’m seriously considering to drop set ±p as well,
only parse -p on the command line, with +p being the default,
and dropping FPRIVILEGED.

Thanks to RT for noticing and jilles for initial follow-up
discussion, as well as Chet Ramey for doing the sane/secure
thing instead of following Debian.
This commit is contained in:
tg 2013-08-23 14:07:39 +00:00
parent 75c00ebaae
commit fda010d8de
6 changed files with 25 additions and 18 deletions

View File

@ -1,5 +1,5 @@
#!/bin/sh
srcversion='$MirOS: src/bin/mksh/Build.sh,v 1.645 2013/08/10 13:44:25 tg Exp $'
srcversion='$MirOS: src/bin/mksh/Build.sh,v 1.646 2013/08/23 14:07:32 tg Exp $'
#-
# Copyright (c) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
# 2011, 2012, 2013
@ -1598,7 +1598,7 @@ else
#define EXTERN
#define MKSH_INCLUDES_ONLY
#include "sh.h"
__RCSID("$MirOS: src/bin/mksh/Build.sh,v 1.645 2013/08/10 13:44:25 tg Exp $");
__RCSID("$MirOS: src/bin/mksh/Build.sh,v 1.646 2013/08/23 14:07:32 tg Exp $");
int main(void) { printf("Hello, World!\n"); return (0); }
EOF
case $cm in
@ -2113,7 +2113,7 @@ addsrcs USE_PRINTF_BUILTIN printf.c
test 1 = "$USE_PRINTF_BUILTIN" && add_cppflags -DMKSH_PRINTF_BUILTIN
test 1 = "$HAVE_CAN_VERB" && CFLAGS="$CFLAGS -verbose"
test -n "$LDSTATIC" && add_cppflags -DMKSH_OPTSTATIC
add_cppflags -DMKSH_BUILD_R=481
add_cppflags -DMKSH_BUILD_R=483
$e $bi$me: Finished configuration testing, now producing output.$ao

View File

@ -1,4 +1,4 @@
# $MirOS: src/bin/mksh/Makefile,v 1.124 2013/08/10 13:44:26 tg Exp $
# $MirOS: src/bin/mksh/Makefile,v 1.125 2013/08/23 14:07:33 tg Exp $
#-
# Copyright (c) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
# 2011, 2012, 2013
@ -54,7 +54,7 @@ CPPFLAGS+= -DMKSH_ASSUME_UTF8 -DMKSH_DISABLE_DEPRECATED \
-DHAVE_SETGROUPS=1 -DHAVE_STRERROR=0 -DHAVE_STRSIGNAL=0 \
-DHAVE_STRLCPY=1 -DHAVE_FLOCK_DECL=1 -DHAVE_REVOKE_DECL=1 \
-DHAVE_SYS_ERRLIST_DECL=1 -DHAVE_SYS_SIGLIST_DECL=1 \
-DHAVE_PERSISTENT_HISTORY=1 -DMKSH_BUILD_R=481
-DHAVE_PERSISTENT_HISTORY=1 -DMKSH_BUILD_R=483
CPPFLAGS+= -D${${PROG:L}_tf:C/(Mir${MAN:E}{0,1}){2}/4/:S/x/mksh_BUILD/:U}
COPTS+= -std=c99 -Wall
.endif

View File

@ -1,4 +1,4 @@
# $MirOS: src/bin/mksh/check.t,v 1.630 2013/08/16 10:58:59 tg Exp $
# $MirOS: src/bin/mksh/check.t,v 1.631 2013/08/23 14:07:34 tg Exp $
# $OpenBSD: bksl-nl.t,v 1.2 2001/01/28 23:04:56 niklas Exp $
# $OpenBSD: history.t,v 1.5 2001/01/28 23:04:56 niklas Exp $
# $OpenBSD: read.t,v 1.3 2003/03/10 03:48:16 david Exp $
@ -31,7 +31,7 @@
# http://www.freebsd.org/cgi/cvsweb.cgi/src/tools/regression/bin/test/regress.sh?rev=HEAD
expected-stdout:
@(#)MIRBSD KSH R48 2013/08/16
@(#)MIRBSD KSH R48 2013/08/23
description:
Check version of shell.
stdin:
@ -40,7 +40,7 @@ name: KSH_VERSION
category: shell:legacy-no
---
expected-stdout:
@(#)LEGACY KSH R48 2013/08/16
@(#)LEGACY KSH R48 2013/08/23
description:
Check version of legacy shell.
stdin:

8
misc.c
View File

@ -30,7 +30,7 @@
#include <grp.h>
#endif
__RCSID("$MirOS: src/bin/mksh/misc.c,v 1.214 2013/08/11 14:57:09 tg Exp $");
__RCSID("$MirOS: src/bin/mksh/misc.c,v 1.215 2013/08/23 14:07:36 tg Exp $");
#define KSH_CHVT_FLAG
#ifdef MKSH_SMALL
@ -271,6 +271,7 @@ change_flag(enum sh_flag f, int what, bool newset)
/*XXX this can probably be optimised */
kshegid = kshgid = getgid();
ksheuid = kshuid = getuid();
#if HAVE_SETRESUGID
DO_SETUID(setresgid, (kshegid, kshegid, kshegid));
#if HAVE_SETGROUPS
@ -278,9 +279,8 @@ change_flag(enum sh_flag f, int what, bool newset)
setgroups(1, &kshegid);
#endif
DO_SETUID(setresuid, (ksheuid, ksheuid, ksheuid));
#else
#else /* !HAVE_SETRESUGID */
/* seteuid, setegid, setgid don't EAGAIN on Linux */
ksheuid = kshuid = getuid();
#ifndef MKSH__NO_SETEUGID
seteuid(ksheuid);
#endif
@ -289,7 +289,7 @@ change_flag(enum sh_flag f, int what, bool newset)
setegid(kshegid);
#endif
setgid(kshegid);
#endif
#endif /* !HAVE_SETRESUGID */
} else if ((f == FPOSIX || f == FSH) && newval) {
/* Turning on -o posix or -o sh? */
Flag(FBRACEEXPAND) = 0;

13
mksh.1
View File

@ -1,4 +1,4 @@
.\" $MirOS: src/bin/mksh/mksh.1,v 1.320 2013/08/10 14:11:39 tg Exp $
.\" $MirOS: src/bin/mksh/mksh.1,v 1.321 2013/08/23 14:07:37 tg Exp $
.\" $OpenBSD: ksh.1,v 1.147 2013/06/13 19:43:09 millert Exp $
.\"-
.\" Copyright © 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
@ -74,7 +74,7 @@
.\" with -mandoc, it might implement .Mx itself, but we want to
.\" use our own definition. And .Dd must come *first*, always.
.\"
.Dd $Mdocdate: August 10 2013 $
.Dd $Mdocdate: August 23 2013 $
.\"
.\" Check which macro package we use, and do other -mdoc setup.
.\"
@ -366,6 +366,13 @@ parameter after subjecting it to parameter, command, arithmetic and tilde
substitution; if unset or empty, the user mkshrc profile is processed;
otherwise, if a file whose name is the substitution result exists,
it is processed; non-existence is silently ignored.
.Pp
The suid profile probably should run
.Ic set +p
unless the shell was explicitly started with
.Fl p .
This isn't easily implemented but a stopgap measure for:
.Pa http://blog.cmpxchg8b.com/2013/08/security\-debianisms.html
.Ss Command syntax
The shell begins parsing its input by removing any backslash-newline
combinations, then breaking it into
@ -6425,7 +6432,7 @@ $ /bin/sleep 666 && echo fubar
.Ed
.Pp
This document attempts to describe
.Nm mksh\ R48
.Nm mksh\ R48c
and up,
compiled without any options impacting functionality, such as
.Dv MKSH_SMALL ,

6
sh.h
View File

@ -164,9 +164,9 @@
#endif
#ifdef EXTERN
__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.668 2013/08/16 10:59:03 tg Exp $");
__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.669 2013/08/23 14:07:39 tg Exp $");
#endif
#define MKSH_VERSION "R48 2013/08/16"
#define MKSH_VERSION "R48 2013/08/23"
/* arithmetic types: C implementation */
#if !HAVE_CAN_INTTYPES
@ -518,7 +518,7 @@ char *ucstrstr(char *, const char *);
#define mkssert(e) do { } while (/* CONSTCOND */ 0)
#endif
#if (!defined(MKSH_BUILDMAKEFILE4BSD) && !defined(MKSH_BUILDSH)) || (MKSH_BUILD_R != 481)
#if (!defined(MKSH_BUILDMAKEFILE4BSD) && !defined(MKSH_BUILDSH)) || (MKSH_BUILD_R != 483)
#error Must run Build.sh to compile this.
extern void thiswillneverbedefinedIhope(void);
int